Re: [PATCH v2 1/3] drm/armada: fbdev: Calculate buffer geometry with format helpers

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The newly added `check_mul_overflow` check is completely ineffective because it stores the result in a `u64` variable. Since both operands (`pitch` and `sizes->surface_height`) are `u32`, their product can never overflow a 64-bit integer, meaning the check always passes. The `u64` size is then implicitly truncated to 32 bits when passed to `armada_gem_alloc_private_object()`, which expects a `size_t` argument. This completely bypasses the intended overflow protection.
--

commit 9304c78ca806a472129099acb5c200bab2e752a0
Author: Thomas Zimmermann <tzimmermann@suse.de>

drm/armada: fbdev: Calculate buffer geometry with format helpers

This commit replaces the geometry and size calculation in armada's fbdev
emulation with DRM format helpers. It uses a 4CC lookup, pitch calculation,
and allocates the GEM buffer object for the framebuffer memory aligned to
PAGE_SIZE.

> diff --git a/drivers/gpu/drm/armada/armada_fbdev.c b/drivers/gpu/drm/armada/armada_fbdev.c
> index 8bbae94804f8b..f95658091acf2 100644
> --- a/drivers/gpu/drm/armada/armada_fbdev.c
> +++ b/drivers/gpu/drm/armada/armada_fbdev.c
> @@ -45,20 +45,30 @@ int armada_fbdev_driver_fbdev_probe(struct drm_fb_helper *fbh,
>  {
[ ... ]
> +	if (check_mul_overflow(pitch, sizes->surface_height, &size))
> +		return -EINVAL;
> +	size = ALIGN(size, PAGE_SIZE);
> +	if (size < PAGE_SIZE)
> +		return -EINVAL;
>  
> -	size = mode.pitches[0] * mode.height;
>  	obj = armada_gem_alloc_private_object(dev, size);

[Severity: High]
This isn't a bug introduced by this patch, but does this check_mul_overflow()
actually prevent overflow here?

Since pitch and sizes->surface_height are 32-bit unsigned integers, and size
is a u64, the multiplication will never overflow the 64-bit target, so the
check will always pass.

However, when size is passed to armada_gem_alloc_private_object(), which
takes a size_t argument, couldn't this silently truncate to 32 bits since
the Armada DRM driver operates on a 32-bit architecture?

For example, if pitch * height evaluates to 0xFFFFFFFF, ALIGN() expands it
to 0x100000000. The check size < PAGE_SIZE evaluates to false on the u64,
but when passed to armada_gem_alloc_private_object(), the size truncates
to 0, resulting in an undersized buffer allocation.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260611072121.71492-1-tzimmermann@suse.de?part=1