Re: [PATCH v3 1/5] drm: Avoid suspicious operations in drm_fb_dma_get_gem_addr()

[Jani Nikula <jani.nikula@linux.intel.com>]

On Tue, 16 Sep 2025, Andi Shyti <andi.shyti@kernel.org> wrote:
> Hi Krzysztof,
>
> On Tue, Sep 16, 2025 at 06:33:00AM +0000, Krzysztof Karas 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. Return 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.
>> 
>> Fixes: 8c30eecc6769 ("drm/gem: rename struct drm_gem_dma_object.{paddr => dma_addr}")
>
> This doesn't need the Fixes tag as it's a very unlikely thing to
> happen.
>
>> Cc: Danilo Krummrich <dakr@redhat.com>
>> Cc: <stable@vger.kernel.org> # v6.1+
>> Reviewed-by: Sebastian Brzezinka <sebastian.brzezinka@intel.com>
>> Signed-off-by: Krzysztof Karas <krzysztof.karas@intel.com>
>> ---
>>  drivers/gpu/drm/drm_fb_dma_helper.c | 7 +++++--
>>  1 file changed, 5 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..00aaad648a33 100644
>> --- a/drivers/gpu/drm/drm_fb_dma_helper.c
>> +++ b/drivers/gpu/drm/drm_fb_dma_helper.c
>> @@ -85,6 +85,9 @@ dma_addr_t drm_fb_dma_get_gem_addr(struct drm_framebuffer *fb,
>>  	u32 block_start_y;
>>  	u32 num_hblocks;
>>  
>> +	if (block_w == 0 || block_h == 0)
>> +		return 0;
>
> This can't go unnoticed, you make the analyzer happy but you
> create bigger issues by silently returning '0'.
>
> If you are really concerned you can place here a BUG_ON or
> WARN_ON_ONCE.

Never BUG* though.

>
> Andi
>
>> +
>>  	obj = drm_fb_dma_get_gem_obj(fb, plane);
>>  	if (!obj)
>>  		return 0;

-- 
Jani Nikula, Intel