Re: [PATCH v2] media: mali-c55: fix integer overflow in scaler factor calculation

[Jacopo Mondi <jacopo.mondi@ideasonboard.com>]

Hi David

On Sat, May 30, 2026 at 11:02:40AM +0100, David CARLIER wrote:
> Hi Jacopo,
>
>   On Sat, May 30, 2026 at 10:55:59AM +0200, Jacopo Mondi wrote:
>   > Have you hit this issue ?
>
>   Not on hardware, I found it by code analysis. The sink format is clamped to
>   8192 and crop is clamped against the sink, so crop->width can reach
>   4096+, where (crop << 20) overflows 32 bits before landing in the u64.
>   I don't have a >=4096 source to reproduce on, but it's provable from the
>   operand widths and the clamp. UHD (3840) is just under; 4096 gives a
>   zero increment, wider values a garbage one.
>
>   > Could we maybe first do the crop/scale division and then do the Q4.20
>   > conversion ? We could maybe save the below do_div() [...]
>
>   I don't think we can - dividing first loses the fraction the Q4.20
>   factor is there to keep. E.g. crop=4096, scale=1920:
>
>     correct:      4096 * 2^20 / 1920 = 2236962  (~2.133)
>     divide-first: (4096 / 1920) << 20 = 2097152  (2.0)   -> ~6.7% off
>
>   So the multiply has to come first, and that pushes the numerator up to
>   8192 * 2^20 = 2^33, which needs a 64-bit divide either way. BIT_ULL()
>   just does the existing multiply in 64-bit. Happy to switch do_div() to
>   div_u64() if you prefer, but that's orthogonal.

Oh yes you're right, I think using ULL is certainly better.

Reviewed-by: Jacopo Mondi <jacopo.mondi@ideasonboard.com>

Thanks
  j



>
> Cheers !