On 4/14/26 07:08, Werner Kasselman wrote:
> amdgpu_gem_align_pitch() is passed u32 width and cpp from dumb buffer
> creation but uses signed int internally. The round-up add and the
> aligned * cpp multiplication can overflow, returning zero or a negative
> pitch. A zero pitch propagates to a zero-sized GEM object allocation
> that reaches userspace via DRM_IOCTL_MODE_CREATE_DUMB.
>
> Switch the helper to unsigned int and use check_add_overflow() /
> check_mul_overflow() so wraparound returns zero. Reject a zero pitch
> or size in amdgpu_mode_dumb_create() rather than allocating a zero-
> byte BO.
>
> Fixes: 8e911ab770f7 ("drm: amdgpu: Replace drm_fb_get_bpp_depth() with
> drm_format_plane_cpp()")
> Cc: [email protected]
The patch looks valid to me, but I think we can drop this CC: stable.
Since dump buffers are a kernel only interface the fix has no practical
relevance and is only of cosmetic nature.
Regards,
Christian.
> Signed-off-by: Werner Kasselman <[email protected]>
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 25 +++++++++++++++++--------
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index a6107109a2b8..0d9309f792a4 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -27,6 +27,7 @@
> */
> #include <linux/ktime.h>
> #include <linux/module.h>
> +#include <linux/overflow.h>
> #include <linux/pagemap.h>
> #include <linux/pci.h>
> #include <linux/dma-buf.h>
> @@ -1223,13 +1224,14 @@ int amdgpu_gem_list_handles_ioctl(struct drm_device
> *dev, void *data,
> return ret;
> }
>
> -static int amdgpu_gem_align_pitch(struct amdgpu_device *adev,
> - int width,
> - int cpp,
> - bool tiled)
> +static unsigned int amdgpu_gem_align_pitch(struct amdgpu_device *adev,
> + unsigned int width,
> + unsigned int cpp,
> + bool tiled)
> {
> - int aligned = width;
> - int pitch_mask = 0;
> + unsigned int aligned = width;
> + unsigned int pitch_mask = 0;
> + unsigned int pitch;
>
> switch (cpp) {
> case 1:
> @@ -1244,9 +1246,12 @@ static int amdgpu_gem_align_pitch(struct amdgpu_device
> *adev,
> break;
> }
>
> - aligned += pitch_mask;
> + if (check_add_overflow(aligned, pitch_mask, &aligned))
> + return 0;
> aligned &= ~pitch_mask;
> - return aligned * cpp;
> + if (check_mul_overflow(aligned, cpp, &pitch))
> + return 0;
> + return pitch;
> }
>
> int amdgpu_mode_dumb_create(struct drm_file *file_priv,
> @@ -1273,8 +1278,12 @@ int amdgpu_mode_dumb_create(struct drm_file *file_priv,
>
> args->pitch = amdgpu_gem_align_pitch(adev, args->width,
> DIV_ROUND_UP(args->bpp, 8), 0);
> + if (!args->pitch)
> + return -EINVAL;
> args->size = (u64)args->pitch * args->height;
> args->size = ALIGN(args->size, PAGE_SIZE);
> + if (!args->size)
> + return -EINVAL;
> domain = amdgpu_bo_get_preferred_domain(adev,
> amdgpu_display_supported_domains(adev, flags));
> r = amdgpu_gem_object_create(adev, args->size, 0, domain, flags,
