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 >