Re: [PATCH] drm/amd/display: avoid 64-bit division

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] Missing 64-bit cast leads to 32-bit multiplication overflow in numerator calculation.
- [High] 32-bit overflow in `modulo_hz * 100` causes incorrect divisor and potential divide-by-zero.
- [Medium] `ASSERT` logic is broken due to 32-bit integer overflow.
--

commit 9cda336bf5ea701c3d123b38512793b3554f5c3b
Author: Arnd Bergmann <arnd@arndb.de>

drm/amd/display: avoid 64-bit division

This commit avoids 64-bit division on 32-bit targets in DCE clock source
calculations by replacing open-coded division with div_u64().

> diff --git a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> index 83dcfae6c3c0d..ed9a6733d3457 100644
> --- a/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> +++ b/drivers/gpu/drm/amd/display/dc/dce/dce_clock_source.c
> @@ -1229,9 +1229,9 @@ static bool get_dp_dto_frequency_100hz(
>  			 */
>  			modulo_hz = REG_READ(MODULO[inst]);
>  			if (modulo_hz) {
> -				temp = div_u64((uint64_t)clock_hz * dp_dto_ref_khz * 10, modulo_hz);
> -				ASSERT(temp / 100 <= 0xFFFFFFFFUL);
> -				*pixel_clk_100hz = (unsigned int)(temp / 100);
> +				temp = clock_hz * dp_dto_ref_khz * 10;

[Severity: High]
Does this drop a required 64-bit cast, leading to a 32-bit multiplication
overflow?

Both clock_hz and dp_dto_ref_khz are 32-bit unsigned integers representing
frequencies. dp_dto_ref_khz is typically around 600,000 (600 MHz in kHz),
and clock_hz can also be extremely large. Their product easily exceeds the
32-bit maximum.

Without the (uint64_t) cast that was present in the original code, the
multiplication is performed entirely using 32-bit arithmetic. The result
overflows, wraps around, and is then implicitly cast to the 64-bit temp
variable, resulting in an incorrect numerator.

> +				ASSERT(temp <= INT_MAX * modulo_hz * 100);

[Severity: Medium]
Can this expression overflow 32-bit integer limits and break the assertion
logic?

The expression INT_MAX * modulo_hz * 100 is evaluated using 32-bit integer
arithmetic. Since INT_MAX is already nearly half of the maximum 32-bit
unsigned value, multiplying it by 100 and any typical modulo_hz frequency
will overflow 32-bit limits multiple times, producing an arbitrary wrapped
value.

The 64-bit temp is then compared against this 32-bit value.

> +				*pixel_clk_100hz = div_u64(temp, modulo_hz * 100);

[Severity: High]
Does modulo_hz * 100 overflow, resulting in an incorrect divisor or a
potential divide-by-zero?

modulo_hz is a 32-bit unsigned integer representing a clock frequency
(often around 600 MHz). Multiplying it by 100 evaluates as a 32-bit
operation, which exceeds UINT_MAX and wraps around.

The div_u64() function expects a u32 divisor, so it uses this truncated,
wrapped value instead of the intended divisor.

If modulo_hz happens to be exactly 1073741824 (about 1.07 GHz),
modulo_hz * 100 wraps to exactly 0, which would trigger a divide-by-zero
kernel panic in div_u64().

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619082313.3583603-1-arnd@kernel.org?part=1