On Thu, 06 Nov 2025, Krzysztof Karas <[email protected]> wrote:
> There are two unsafe scenarios in that function:
>  1) drm_format_info_block_width/height() may return 0 and cause
> division by 0 down the line. Warn early if any of these values
> are 0.
>  2) dma_addr calculations are carried out using 32-bit
> arithmetic, which could cause a truncation of the values
> before they are extended to 64 bits. Cast one of the operands
> to dma_addr_t, so 64-bit arithmetic is used.

Lists like this are an indication it should be multiple patches. The
only thing in common here is the function.

>
> Signed-off-by: Krzysztof Karas <[email protected]>
> ---
>
> v4:
> * use WARN_ON_ONCE() instead of returning early (Andi)

I don't get it. Why are we allowing the oops if we check it?

>
>  drivers/gpu/drm/drm_fb_dma_helper.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/gpu/drm/drm_fb_dma_helper.c 
> b/drivers/gpu/drm/drm_fb_dma_helper.c
> index fd71969d2fb1..da1627dfd9f8 100644
> --- a/drivers/gpu/drm/drm_fb_dma_helper.c
> +++ b/drivers/gpu/drm/drm_fb_dma_helper.c
> @@ -85,6 +85,8 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer 
> *fb,
>       u32 block_start_y;
>       u32 num_hblocks;
>  
> +     WARN_ON_ONCE(block_w == 0 || block_h == 0);
> +
>       obj = drm_fb_dma_get_gem_obj(fb, plane);
>       if (!obj)
>               return 0;
> @@ -101,8 +103,8 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer 
> *fb,
>       block_start_y = (sample_y / block_h) * block_h;
>       num_hblocks = sample_x / block_w;
>  
> -     dma_addr += fb->pitches[plane] * block_start_y;
> -     dma_addr += block_size * num_hblocks;
> +     dma_addr += (dma_addr_t)fb->pitches[plane] * block_start_y;
> +     dma_addr += (dma_addr_t)block_size * num_hblocks;
>  
>       return dma_addr;
>  }

-- 
Jani Nikula, Intel

Reply via email to