Re: [PATCH] drm/amdgpu: reject oversized IBs on rings with 20-bit size fields

Alex Deucher <[email protected]> Thu, 30 Jul 2026 09:20:43 -0400
Newsgroups org.freedesktop.lists.amd-gfx
Message-ID <CADnq5_N07acyyUs23y9EJser=xzOX4u=g4OVwH+fp882cm1wvQ@mail.gmail.com>
On Thu, Jul 30, 2026 at 6:08 AM Candice Li <[email protected]> wrote:
>
> PM4 INDIRECT_BUFFER control words encode IB size in 20 bits.  On GFX
> rings amdgpu_cs_p2_ib() passed user-supplied ib_bytes through to
> ib->length_dw without a limit, and ring_emit_ib() ORed length_dw
> directly into the control dword.

As a follow on patch, it would be good to mask the length_dw in the
gfx emit_ib() functions as well just to be safe.

>
> Reject IB submissions whose length exceeds the hardware maximum in
> amdgpu_cs_p2_ib(), but only for rings whose emit_ib packet format
> documents a 20-bit size field (GFX, compute and SDMA).
>
> Signed-off-by: Candice Li <[email protected]>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index afc1c631d55aa6..0d06b6d166c576 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -42,6 +42,25 @@
>  #include "amdgpu_ras.h"
>  #include "amdgpu_hmm.h"
>
> +/*
> + * Maximum IB length (dwords) for rings whose emit_ib packet format
> + * documents a 20-bit size field, e.g. GFX PM4 INDIRECT_BUFFER ([19:0])
> + * and SDMA indirect packets.
> + */
> +#define AMDGPU_IB_PACKET_SIZE_MAX_DW   0xFFFFF

#define AMDGPU_GFX_SDMA_IB_PACKET_SIZE_MAX_DW   0xFFFFF
#define AMDGPU_MM_IB_PACKET_SIZE_MAX_DW   0x7FFFF0

> +
> +static bool amdgpu_cs_ib_has_packet_length_limit(enum amdgpu_ring_type type)
> +{

I would make this function return the max IB size is DWs.

> +       switch (type) {
> +       case AMDGPU_RING_TYPE_GFX:
> +       case AMDGPU_RING_TYPE_COMPUTE:
> +       case AMDGPU_RING_TYPE_SDMA:

I suspect VPE also should be covered by this case since it's based on SDMA.

return AMDGPU_GFX_SDMA_IB_PACKET_SIZE_MAX_DW here.

> +               return true;
> +       default:

and

return AMDGPU_MM_IB_PACKET_SIZE_MAX_DW here.

> +               return false;
> +       }
> +}
> +
>  static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p,
>                                  struct amdgpu_device *adev,
>                                  struct drm_file *filp,
> @@ -340,7 +359,6 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser *p,
>
>         job = p->jobs[r];
>         ring = amdgpu_job_ring(job);
> -       ib = &job->ibs[job->num_ibs++];
>
>         /* submissions to kernel queues are disabled */
>         if (ring->no_user_submission)
> @@ -369,6 +387,12 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser *p,
>                         return -EINVAL;
>         }
>
> +       if (amdgpu_cs_ib_has_packet_length_limit(ring->funcs->type) &&
> +           chunk_ib->ib_bytes / 4 > AMDGPU_IB_PACKET_SIZE_MAX_DW)

Then check the max size returned by the above function here.

Alex

> +               return -EINVAL;
> +
> +       ib = &job->ibs[job->num_ibs++];
> +
>         if (chunk_ib->flags & AMDGPU_IB_FLAG_PREAMBLE)
>                 job->preamble_status |= AMDGPU_PREAMBLE_IB_PRESENT;
>
> --
> 2.25.1
>