Re: [PATCH v2 1/2] drm/amdgpu: reject IB addresses with reserved byte-swap bits

["Christian König" <christian.koenig@amd.com>]

On 4/24/26 16:08, John B. Moore wrote:
> Reject IB GPU addresses with bits [1:0] set early in the CS parser,
> before they reach ring emission callbacks. On legacy AMD hardware
> (pre-amdgpu era), these two bits encoded byte-swap mode for IB memory
> fetches. That feature was dropped on all hardware that amdgpu supports,
> but the ring emission paths still contain BUG_ON(addr & 0x3) assertions
> that crash the kernel if userspace submits a misaligned IB address.
> 
> Add an early check in amdgpu_cs_p2_ib() to reject such submissions
> with -EINVAL before the IB is allocated, and a defense-in-depth
> WARN_ON_ONCE in amdgpu_ib_schedule() to catch any that slip through
> from other code paths.
> 
> Fixes: b0635e808290 ("drm/amdgpu: implement GFX 9.0 support (v2)")
> Cc: stable@vger.kernel.org
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> ---
>  drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c |  8 ++++++++
>  drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c | 10 ++++++++++
>  2 files changed, 18 insertions(+)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> index 10d8dcc3a..53f537f3e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_cs.c
> @@ -379,6 +379,14 @@ static int amdgpu_cs_p2_ib(struct amdgpu_cs_parser *p,
>  	if (chunk_ib->flags & AMDGPU_IB_FLAG_PREAMBLE)
>  		job->preamble_status |= AMDGPU_PREAMBLE_IB_PRESENT;
>  
> +	/* Reject IB addresses with reserved byte-swap bits set.
> +	 * On legacy HW (pre-amdgpu), bits [1:0] encoded byte-swap mode
> +	 * for IB fetches. That feature is deprecated on all HW that
> +	 * amdgpu supports, so these bits must be zero.
> +	 */
> +	if (chunk_ib->va_start & 0x3)
> +		return -EINVAL;
> +
>  	r =  amdgpu_ib_get(p->adev, vm, ring->funcs->parse_cs ?
>  			   chunk_ib->ib_bytes : 0,
>  			   AMDGPU_IB_POOL_DELAYED, ib);
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> index f1ed4a436..3111d2c7e 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ib.c
> @@ -272,6 +272,16 @@ int amdgpu_ib_schedule(struct amdgpu_ring *ring, unsigned int num_ibs,
>  	for (i = 0; i < num_ibs; ++i) {
>  		ib = &ibs[i];
>  
> +		/* Defense-in-depth: the CS parser rejects misaligned IB
> +		 * addresses, but catch any that slip through before they
> +		 * hit BUG_ON(addr & 0x3) in ring emission callbacks.
> +		 */
> +		if (WARN_ON_ONCE(ib->gpu_addr & 0x3)) {
> +			r = -EINVAL;
> +			amdgpu_ring_undo(ring);
> +			goto free_fence;
> +		}
> +

Please drop that chunk. Apart from that the patch looks good to me.

Regards,
Christian.

>  		if (job && ring->funcs->emit_frame_cntl) {
>  			if (secure != !!(ib->flags & AMDGPU_IB_FLAGS_SECURE)) {
>  				amdgpu_ring_emit_frame_cntl(ring, false, secure);