[PATCH v2] drm/amd/ras: Fix type size of remainder argument

[PATCH v2] drm/amd/ras: Fix type size of remainder argument

[Kees Cook]

Forcing an int to be dereferenced at uint64_t for div64_u64_rem() runs
the risk of endian confusion and stack overflowing writes. Seen while
preparing to enable -Warray-bounds globally:

In file included from ../arch/x86/include/asm/processor.h:35,
                 from ../include/linux/sched.h:13,
                 from ../include/linux/ratelimit.h:6,
                 from ../include/linux/dev_printk.h:16,
                 from ../drivers/gpu/drm/amd/amdgpu/../ras/ras_mgr/ras_sys.h:29,
                 from ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras.h:27,
                 from ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c:24:
In function 'div64_u64_rem',
    inlined from 'ras_core_convert_timestamp_to_time' at ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c:72:9:
../include/linux/math64.h:56:20: error: array subscript 'u64 {aka long long unsigned int}[0]' is partly outside array bounds of 'int[1]' [-Werror=array-bounds=]
   56 |         *remainder = dividend % divisor;
      |         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c: In function 'ras_core_convert_timestamp_to_time':
../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c:70:19: note: object 'remaining_seconds' of size 4
   70 |         int days, remaining_seconds;
      |                   ^~~~~~~~~~~~~~~~~

Use a 64-bit type for the remainder calculation, but leave
remaining_seconds as 32-bit to avoid 64-bit division later. The value of
remainder will always be less than seconds_per_day, so there's no
truncation risk.

Fixes: ace232eff50e ("drm/amdgpu: Add ras module files into amdgpu")
Signed-off-by: Kees Cook <kees@kernel.org>
---
 v2: use temp u64 to avoid 64-bit division later
 v1: https://lore.kernel.org/lkml/20260225024716.work.043-kees@kernel.org/
Cc: Alex Deucher <alexander.deucher@amd.com>
Cc: "Christian König" <christian.koenig@amd.com>
Cc: David Airlie <airlied@gmail.com>
Cc: Simona Vetter <simona@ffwll.ch>
Cc: YiPeng Chai <YiPeng.Chai@amd.com>
Cc: Tao Zhou <tao.zhou1@amd.com>
Cc: Hawking Zhang <Hawking.Zhang@amd.com>
Cc: <amd-gfx@lists.freedesktop.org>
Cc: <dri-devel@lists.freedesktop.org>
---
 drivers/gpu/drm/amd/ras/rascore/ras_core.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/ras/rascore/ras_core.c b/drivers/gpu/drm/amd/ras/rascore/ras_core.c
index 01122b55c98a..02bbee64a5bd 100644
--- a/drivers/gpu/drm/amd/ras/rascore/ras_core.c
+++ b/drivers/gpu/drm/amd/ras/rascore/ras_core.c
@@ -62,14 +62,16 @@ int ras_core_convert_timestamp_to_time(struct ras_core_context *ras_core,
 			uint64_t timestamp, struct ras_time *tm)
 {
 	int days_in_month[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
-	uint64_t month = 0, day = 0, hour = 0, minute = 0, second = 0;
+	uint64_t month = 0, day = 0, hour = 0, minute = 0, second = 0, remainder;
 	uint32_t year = 0;
 	int seconds_per_day = 24 * 60 * 60;
 	int seconds_per_hour = 60 * 60;
 	int seconds_per_minute = 60;
 	int days, remaining_seconds;
 
-	days = div64_u64_rem(timestamp, seconds_per_day, (uint64_t *)&remaining_seconds);
+	days = div64_u64_rem(timestamp, seconds_per_day, &remainder);
+	/* remainder will always be less than seconds_per_day. */
+	remaining_seconds = remainder;
 
 	/* utc_timestamp follows the Unix epoch */
 	year = 1970;
-- 
2.34.1

Re: [PATCH v2] drm/amd/ras: Fix type size of remainder argument

[Alex Deucher]

Applied.  Thanks!

Alex

On Wed, Feb 25, 2026 at 1:04 PM Kees Cook <kees@kernel.org> wrote:
>
> Forcing an int to be dereferenced at uint64_t for div64_u64_rem() runs
> the risk of endian confusion and stack overflowing writes. Seen while
> preparing to enable -Warray-bounds globally:
>
> In file included from ../arch/x86/include/asm/processor.h:35,
>                  from ../include/linux/sched.h:13,
>                  from ../include/linux/ratelimit.h:6,
>                  from ../include/linux/dev_printk.h:16,
>                  from ../drivers/gpu/drm/amd/amdgpu/../ras/ras_mgr/ras_sys.h:29,
>                  from ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras.h:27,
>                  from ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c:24:
> In function 'div64_u64_rem',
>     inlined from 'ras_core_convert_timestamp_to_time' at ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c:72:9:
> ../include/linux/math64.h:56:20: error: array subscript 'u64 {aka long long unsigned int}[0]' is partly outside array bounds of 'int[1]' [-Werror=array-bounds=]
>    56 |         *remainder = dividend % divisor;
>       |         ~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~
> ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c: In function 'ras_core_convert_timestamp_to_time':
> ../drivers/gpu/drm/amd/amdgpu/../ras/rascore/ras_core.c:70:19: note: object 'remaining_seconds' of size 4
>    70 |         int days, remaining_seconds;
>       |                   ^~~~~~~~~~~~~~~~~
>
> Use a 64-bit type for the remainder calculation, but leave
> remaining_seconds as 32-bit to avoid 64-bit division later. The value of
> remainder will always be less than seconds_per_day, so there's no
> truncation risk.
>
> Fixes: ace232eff50e ("drm/amdgpu: Add ras module files into amdgpu")
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
>  v2: use temp u64 to avoid 64-bit division later
>  v1: https://lore.kernel.org/lkml/20260225024716.work.043-kees@kernel.org/
> Cc: Alex Deucher <alexander.deucher@amd.com>
> Cc: "Christian König" <christian.koenig@amd.com>
> Cc: David Airlie <airlied@gmail.com>
> Cc: Simona Vetter <simona@ffwll.ch>
> Cc: YiPeng Chai <YiPeng.Chai@amd.com>
> Cc: Tao Zhou <tao.zhou1@amd.com>
> Cc: Hawking Zhang <Hawking.Zhang@amd.com>
> Cc: <amd-gfx@lists.freedesktop.org>
> Cc: <dri-devel@lists.freedesktop.org>
> ---
>  drivers/gpu/drm/amd/ras/rascore/ras_core.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/ras/rascore/ras_core.c b/drivers/gpu/drm/amd/ras/rascore/ras_core.c
> index 01122b55c98a..02bbee64a5bd 100644
> --- a/drivers/gpu/drm/amd/ras/rascore/ras_core.c
> +++ b/drivers/gpu/drm/amd/ras/rascore/ras_core.c
> @@ -62,14 +62,16 @@ int ras_core_convert_timestamp_to_time(struct ras_core_context *ras_core,
>                         uint64_t timestamp, struct ras_time *tm)
>  {
>         int days_in_month[] = {31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31};
> -       uint64_t month = 0, day = 0, hour = 0, minute = 0, second = 0;
> +       uint64_t month = 0, day = 0, hour = 0, minute = 0, second = 0, remainder;
>         uint32_t year = 0;
>         int seconds_per_day = 24 * 60 * 60;
>         int seconds_per_hour = 60 * 60;
>         int seconds_per_minute = 60;
>         int days, remaining_seconds;
>
> -       days = div64_u64_rem(timestamp, seconds_per_day, (uint64_t *)&remaining_seconds);
> +       days = div64_u64_rem(timestamp, seconds_per_day, &remainder);
> +       /* remainder will always be less than seconds_per_day. */
> +       remaining_seconds = remainder;
>
>         /* utc_timestamp follows the Unix epoch */
>         year = 1970;
> --
> 2.34.1
>

Claude review: drm/amd/ras: Fix type size of remainder argument

[Claude Code Review Bot]

Overall Series Review

Subject: drm/amd/ras: Fix type size of remainder argument
Author: Kees Cook <kees@kernel.org>
Patches: 2
Reviewed: 2026-02-27T13:07:15.223220

---

This is a single-patch fix from Kees Cook addressing a real type-safety bug found while enabling `-Warray-bounds` globally. The original code passes a pointer to a 4-byte `int` cast to `uint64_t *` into `div64_u64_rem()`, which writes 8 bytes through that pointer. This causes a stack buffer overflow of 4 bytes and, on big-endian architectures, also produces a wrong value (the upper 32 bits of the remainder would be written over `remaining_seconds` while the meaningful lower 32 bits go to whatever is adjacent on the stack).

The fix is correct, clean, and minimal. **Recommend: Accept.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amd/ras: Fix type size of remainder argument

[Claude Code Review Bot]

Patch Review

**Problem analysis:** The original code at line 72:
```c
int days, remaining_seconds;
days = div64_u64_rem(timestamp, seconds_per_day, (uint64_t *)&remaining_seconds);
```
`div64_u64_rem()` takes a `u64 *remainder` parameter (confirmed in `include/linux/math64.h:54`) and writes 8 bytes through it. But `remaining_seconds` is `int` (4 bytes). The cast `(uint64_t *)&remaining_seconds` silences the compiler but creates two real bugs:
1. **Stack corruption**: 4 bytes of adjacent stack are overwritten.
2. **Endianness bug**: On big-endian systems, the meaningful lower 32 bits of the remainder end up in the wrong half.

**Fix analysis:** The patch introduces a properly-typed `u64 remainder` temporary:
```c
uint64_t month = 0, day = 0, hour = 0, minute = 0, second = 0, remainder;
...
days = div64_u64_rem(timestamp, seconds_per_day, &remainder);
/* remainder will always be less than seconds_per_day. */
remaining_seconds = remainder;
```

This is the right approach for v2. The v1 presumably changed `remaining_seconds` itself to `u64`, which would then require `do_div()` or similar at the later division sites (lines 98-100). By using a temporary and assigning back to `int`, the subsequent arithmetic on `remaining_seconds` stays as native `int` division — no 64-bit division helpers needed.

**Comment on the comment:** The inline comment `/* remainder will always be less than seconds_per_day. */` is helpful — it justifies the narrowing assignment. `seconds_per_day` is 86400, well within `int` range, so the truncation is safe.

**One minor observation:** `days` is also `int` (32-bit) and receives the return value of `div64_u64_rem()`, which returns `u64`. For a Unix timestamp, this quotient is the number of days since epoch — currently ~20,500 and growing. An `int` can hold up to ~2.1 billion days, so this is safe in practice, but it's a preexisting issue and outside the scope of this patch.

**Verdict:** The fix is correct, well-motivated, and minimal. The Fixes tag correctly references the commit that introduced the buggy code. No issues found.

Reviewed-by quality: **Good to merge as-is.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amd/ras: Fix type size of remainder argument

[Claude Code Review Bot]

Overall Series Review

Subject: drm/amd/ras: Fix type size of remainder argument
Author: Kees Cook <kees@kernel.org>
Patches: 2
Reviewed: 2026-02-27T14:18:28.692244

---

This is a single-patch fix from Kees Cook that addresses a real type-safety bug in `ras_core_convert_timestamp_to_time()`. The function passes an `int *` cast to `uint64_t *` into `div64_u64_rem()`, which expects a `u64 *remainder` parameter. This is a legitimate problem: on 64-bit platforms the store through the `uint64_t *` pointer will write 8 bytes into a 4-byte `int`, corrupting the adjacent stack variable, and on big-endian systems the wrong half of the 64-bit value would land in the `int`. The fix is straightforward and correct. The patch is well-motivated by a real compiler warning (`-Warray-bounds`) and has a proper `Fixes:` tag.

**Verdict: The patch looks good and should be accepted.**

---
Generated by Claude Code Patch Reviewer

Claude review: drm/amd/ras: Fix type size of remainder argument

[Claude Code Review Bot]

Patch Review

**Problem analysis:** The original code at line 70-72 of `ras_core.c` declares:
```c
int days, remaining_seconds;
...
days = div64_u64_rem(timestamp, seconds_per_day, (uint64_t *)&remaining_seconds);
```

`div64_u64_rem()` has the signature (from `include/linux/math64.h:54`):
```c
static inline u64 div64_u64_rem(u64 dividend, u64 divisor, u64 *remainder)
```

Writing a `u64` (8 bytes) through a pointer to a 4-byte `int` is an out-of-bounds stack write. On little-endian x86 this happens to work by luck (the low 4 bytes land correctly, and the high 4 bytes overflow into adjacent stack space), but it's undefined behavior and would produce wrong results on big-endian architectures.

**Fix review:** The patch changes `remaining_seconds` from `int` to `uint64_t` and removes the cast:
```c
+	uint64_t remaining_seconds;
...
+	days = div64_u64_rem(timestamp, seconds_per_day, &remaining_seconds);
```

This is the correct fix. The value stored in `remaining_seconds` (seconds within a day, range 0–86399) fits comfortably in any integer type, so the widening has no functional impact on the subsequent arithmetic at lines 97-100 where `remaining_seconds` is used for division/modulo to compute hours, minutes, and seconds.

**Minor observations (not blocking):**

1. The return type of `div64_u64_rem()` is `u64`, but `days` remains `int`. For timestamp values representable by a `u64`, the number of days could theoretically exceed `INT_MAX` (~5.8 million years), but practically this is fine since the function processes realistic Unix timestamps. Not something this patch needs to address.

2. The `seconds_per_day`, `seconds_per_hour`, and `seconds_per_minute` variables are `int`, and they are passed as the `u64 divisor` argument to `div64_u64_rem()` — this is an implicit widening conversion which is safe and correct. No issue here.

**Reviewed-by: Looks correct.** The fix is minimal, targeted, and solves both the `-Warray-bounds` warning and the underlying type-safety/endianness bug.

---
Generated by Claude Code Patch Reviewer