Re: [PATCH 1/2] drm/amdgpu: fix integer overflow in amdgpu_gem_align_pitch()

[Alex Deucher]

On Tue, Apr 7, 2026 at 3:41 AM Werner Kasselman <werner@xxxxxxxxxx> wrote:
>
> amdgpu_gem_align_pitch() uses signed int for the pitch calculation.

Can you convert amdgpu_gem_align_pitch() to use unsigned ints?  The
width passed to it is always unsigned.  That would simplify things.

Alex

> When alignment rounding pushes the width to a boundary value (e.g.,
> 2^30 for cpp=4), the multiplication 'aligned * cpp' overflows signed
> 32-bit int, producing 0 or a negative value.
>
> The overflow guard in drm_mode_create_dumb() validates width * cpp
> BEFORE the driver callback, but amdgpu_mode_dumb_create() bypasses the
> generic drm_mode_size_dumb() helper and performs its own alignment
> rounding, which can push the pitch past the pre-validated range.
>
> A zero pitch propagates to a zero-size GEM object allocation via
> amdgpu_gem_object_create(). The 0-byte BO passes
> amdgpu_bo_validate_size() (since 0 < man->size) and is returned to
> userspace with a valid handle. This object can then be mmap'd or
> referenced in GPU command submissions, potentially causing out-of-bounds
> access to adjacent slab memory.
>
> DRM_IOCTL_MODE_CREATE_DUMB requires no DRM authentication, so any local
> user with access to /dev/dri/renderD* can trigger this with e.g.
> width=1073741760, bpp=32, height=1.
>
> Add an overflow check in amdgpu_gem_align_pitch() to detect when
> 'aligned * cpp' would exceed INT_MAX, returning 0 in that case. Add
> corresponding checks in amdgpu_mode_dumb_create() to reject pitch=0
> and size=0 with -EINVAL.
>
> The proper long-term fix is to convert amdgpu to use
> drm_mode_size_dumb() which centralizes pitch/size calculation with
> proper overflow guards, as is being done for other drivers in Thomas
> Zimmermann's dumb-buffer series.
>
> Found via AST-based call-graph analysis using sqry.
>
> Fixes: 087451f372bf ("drm/amdgpu: use generic fb helpers instead of setting up AMD own's.")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Werner Kasselman <werner@xxxxxxxxxxx>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 13 +++++++++++++
>  1 file changed, 13 insertions(+)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> index a6107109a2b8..b4341abba20c 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
> @@ -1246,6 +1246,15 @@ static int amdgpu_gem_align_pitch(struct amdgpu_device *adev,
>
>         aligned += pitch_mask;
>         aligned &= ~pitch_mask;
> +
> +       /* Sanity check to avoid integer overflow in aligned * cpp.
> +        * The caller (drm_mode_create_dumb) validates width * cpp fits
> +        * in u32 before alignment, but rounding up can push aligned
> +        * past INT_MAX / cpp, causing signed overflow to 0 or negative.
> +        */
> +       if (aligned > INT_MAX / (cpp ? cpp : 1) || aligned <= 0)
> +               return 0;
> +
>         return aligned * cpp;
>  }
>
> @@ -1273,8 +1282,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,
> --
> 2.43.0
>