AMD General Good point, thanks. I’ll keep this patch focused on CS-side input validation and follow up with a separate series to enforce IB length masking in the ring packet emit paths as defense in depth.
Thanks, Candice -----Original Message----- From: Alex Deucher <[email protected]> Sent: Monday, August 3, 2026 10:00 PM To: Li, Candice <[email protected]> Cc: [email protected]; Deucher, Alexander <[email protected]> Subject: Re: [PATCH v2] drm/amdgpu: reject oversized IBs with per-ring packet limits On Mon, Aug 3, 2026 at 6:33 AM Candice Li <[email protected]> wrote: > > On GFX rings, amdgpu_cs_p2_ib() passed user-supplied ib_bytes through > to ib->length_dw without a limit, while ring_emit_ib() encodes length > into packet fields. Oversized values can corrupt adjacent control bits > and destabilize command submission. > > Add a per-ring IB packet size limit helper and reject command > submissions exceeding the corresponding dword limit before IB > allocation. Use the documented 20-bit limit for GFX/compute/SDMA/VPE, > and apply the MM fallback limit for other ring types. > > Signed-off-by: Candice Li <[email protected]> Would be good to also enforce the ib length mask in the packet functions as well as a follow up patch series. This patch is: Reviewed-by: Alex Deucher <[email protected]> > --- > drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c | 27 +++++++++++++++++++++++++- > 1 file changed, 26 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..617f53f135f353 100644 > --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c > @@ -42,6 +42,26 @@ > #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. > + */ > +#define AMDGPU_GFX_SDMA_IB_PACKET_SIZE_MAX_DW 0xFFFFF > +#define AMDGPU_MM_IB_PACKET_SIZE_MAX_DW 0x7FFFF0 > + > +static u32 amdgpu_cs_ib_packet_size_max_dw(enum amdgpu_ring_type type) > +{ > + switch (type) { > + case AMDGPU_RING_TYPE_GFX: > + case AMDGPU_RING_TYPE_COMPUTE: > + case AMDGPU_RING_TYPE_SDMA: > + case AMDGPU_RING_TYPE_VPE: > + return AMDGPU_GFX_SDMA_IB_PACKET_SIZE_MAX_DW; > + default: > + return AMDGPU_MM_IB_PACKET_SIZE_MAX_DW; > + } > +} > + > static int amdgpu_cs_parser_init(struct amdgpu_cs_parser *p, > struct amdgpu_device *adev, > struct drm_file *filp, > @@ -340,7 +360,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 +388,12 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser *p, > return -EINVAL; > } > > + if (chunk_ib->ib_bytes / 4 > > + amdgpu_cs_ib_packet_size_max_dw(ring->funcs->type)) > + return -EINVAL; > + > + ib = &job->ibs[job->num_ibs++]; > + > if (chunk_ib->flags & AMDGPU_IB_FLAG_PREAMBLE) > job->preamble_status |= AMDGPU_PREAMBLE_IB_PRESENT; > > -- > 2.50.1 >
