[PATCH 1/4] drm/amdgpu/sdma4: replace BUG_ON with WARN_ON_ONCE in fence emission

[PATCH 1/4] drm/amdgpu/sdma4: replace BUG_ON with WARN_ON_ONCE in fence emission

[jbmoore]

From: "John B. Moore" <jbmoore61@gmail.com>

sdma_v4_0_ring_emit_fence() contains two BUG_ON(addr & 0x3) assertions
that verify fence writeback addresses are dword-aligned.  These
assertions can be reached via crafted DRM_IOCTL_AMDGPU_CS submissions
from unprivileged userspace, causing a fatal kernel panic in a
scheduler worker thread.

Replace both BUG_ON() calls with WARN_ON_ONCE() and force-align the
address by clearing the reserved bits.  This logs the condition once
per boot and allows the hardware to proceed without crashing the
kernel.

On all hardware that amdgpu supports, bits [1:0] of ring buffer
addresses are reserved (they historically encoded byte-swap mode on
legacy pre-amdgpu hardware).  A misaligned fence address indicates a
driver bug, but crashing the kernel is never the correct response.

Found by a custom amdgpu DRM ioctl fuzzer.

Fixes: 2130f89ced2c ("drm/amdgpu: add SDMA v4.0 implementation (v2)")
Signed-off-by: John B. Moore <jbmoore61@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
index 8a2a4e618..dcb7e4219 100644
--- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
@@ -889,7 +889,8 @@ static void sdma_v4_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 se
 	/* write the fence */
 	amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_FENCE));
 	/* zero in first two bits */
-	BUG_ON(addr & 0x3);
+	if (WARN_ON_ONCE(addr & 0x3))
+		addr &= ~0x3ULL;
 	amdgpu_ring_write(ring, lower_32_bits(addr));
 	amdgpu_ring_write(ring, upper_32_bits(addr));
 	amdgpu_ring_write(ring, lower_32_bits(seq));
@@ -899,7 +900,8 @@ static void sdma_v4_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 se
 		addr += 4;
 		amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_FENCE));
 		/* zero in first two bits */
-		BUG_ON(addr & 0x3);
+		if (WARN_ON_ONCE(addr & 0x3))
+			addr &= ~0x3ULL;
 		amdgpu_ring_write(ring, lower_32_bits(addr));
 		amdgpu_ring_write(ring, upper_32_bits(addr));
 		amdgpu_ring_write(ring, upper_32_bits(seq));
-- 
2.43.0

Re: [PATCH 1/4] drm/amdgpu/sdma4: replace BUG_ON with WARN_ON_ONCE in fence emission

[Christian König]

On 4/26/26 23:52, jbmoore wrote:
> From: "John B. Moore" <jbmoore61@gmail.com>
> 
> sdma_v4_0_ring_emit_fence() contains two BUG_ON(addr & 0x3) assertions
> that verify fence writeback addresses are dword-aligned.  These
> assertions can be reached via crafted DRM_IOCTL_AMDGPU_CS submissions
> from unprivileged userspace, causing a fatal kernel panic in a
> scheduler worker thread.
> 
> Replace both BUG_ON() calls with WARN_ON_ONCE() and force-align the
> address by clearing the reserved bits.  This logs the condition once
> per boot and allows the hardware to proceed without crashing the
> kernel.
> 
> On all hardware that amdgpu supports, bits [1:0] of ring buffer
> addresses are reserved (they historically encoded byte-swap mode on
> legacy pre-amdgpu hardware).  A misaligned fence address indicates a
> driver bug, but crashing the kernel is never the correct response.
> 
> Found by a custom amdgpu DRM ioctl fuzzer.
> 
> Fixes: 2130f89ced2c ("drm/amdgpu: add SDMA v4.0 implementation (v2)")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c | 6 ++++--
>  1 file changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> index 8a2a4e618..dcb7e4219 100644
> --- a/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/sdma_v4_0.c
> @@ -889,7 +889,8 @@ static void sdma_v4_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 se
>  	/* write the fence */
>  	amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_FENCE));
>  	/* zero in first two bits */
> -	BUG_ON(addr & 0x3);
> +	if (WARN_ON_ONCE(addr & 0x3))
> +		addr &= ~0x3ULL;
>  	amdgpu_ring_write(ring, lower_32_bits(addr));
>  	amdgpu_ring_write(ring, upper_32_bits(addr));
>  	amdgpu_ring_write(ring, lower_32_bits(seq));
> @@ -899,7 +900,8 @@ static void sdma_v4_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 se
>  		addr += 4;
>  		amdgpu_ring_write(ring, SDMA_PKT_HEADER_OP(SDMA_OP_FENCE));
>  		/* zero in first two bits */
> -		BUG_ON(addr & 0x3);
> +		if (WARN_ON_ONCE(addr & 0x3))
> +			addr &= ~0x3ULL;

A WARN_ON() should be sufficient here and I don't think we should mask the lower bits.

It is perfectly possible that the lower bits were re-used for some other feature than byte swap.

We should just make sure that the CS IOCTL filters out all invalid submissions since here it is clearly to late to do anything about it.

Regards,
Christian.


>  		amdgpu_ring_write(ring, lower_32_bits(addr));
>  		amdgpu_ring_write(ring, upper_32_bits(addr));
>  		amdgpu_ring_write(ring, upper_32_bits(seq));

[PATCH 2/4] drm/amdgpu/gfx9: replace BUG_ON/BUG with WARN_ON_ONCE in ring emission

[jbmoore]

From: "John B. Moore" <jbmoore61@gmail.com>

Replace all BUG_ON() and BUG() assertions in the gfx_v9_0 ring
emission paths with WARN_ON_ONCE() and graceful recovery.  Ten sites
are converted across wait_reg_mem, gpu_early_init, parse_ind_reg_list,
init_rlc_save_restore_list, kiq_read_clock, emit_ib_gfx,
emit_ib_compute, emit_fence, get_wptr_compute, and set_wptr_compute.

These assertions guard conditions that are either:
- Address alignment checks on a deprecated byte-swap encoding from
  legacy pre-amdgpu hardware (bits [1:0] must be zero), or
- Switch-case defaults that should be unreachable but are better
  handled with dev_err + return -EINVAL than a kernel panic.

Several of the address alignment BUG_ON sites in the IB emission
paths (emit_ib_gfx, emit_ib_compute) are reachable from unprivileged
userspace via crafted DRM_IOCTL_AMDGPU_CS submissions, causing a
fatal kernel panic in a scheduler worker thread.

For address checks, clear the reserved bits and proceed.  For
unreachable switch defaults, log the error and return.  For the
doorbell-only wptr paths, log with WARN_ONCE and return zero /
no-op.  Ring emission callbacks return void, so force-aligning
and proceeding is the accepted pattern.

Found by a custom amdgpu DRM ioctl fuzzer.

Fixes: b1023571479020e9 ("drm/amdgpu: implement GFX 9.0 support (v2)")
Signed-off-by: John B. Moore <jbmoore61@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 50 +++++++++++++++++----------
 1 file changed, 32 insertions(+), 18 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 2eb32f92a..47e81c33d 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -1182,8 +1182,8 @@ static void gfx_v9_0_wait_reg_mem(struct amdgpu_ring *ring, int eng_sel,
 				 WAIT_REG_MEM_FUNCTION(3) |  /* equal */
 				 WAIT_REG_MEM_ENGINE(eng_sel)));
 
-	if (mem_space)
-		BUG_ON(addr0 & 0x3); /* Dword align */
+	if (mem_space && WARN_ON_ONCE(addr0 & 0x3))
+		addr0 &= ~0x3; /* Force dword align */
 	amdgpu_ring_write(ring, addr0);
 	amdgpu_ring_write(ring, addr1);
 	amdgpu_ring_write(ring, ref);
@@ -2107,8 +2107,10 @@ static int gfx_v9_0_gpu_early_init(struct amdgpu_device *adev)
 			return err;
 		break;
 	default:
-		BUG();
-		break;
+		dev_err(adev->dev,
+			"unsupported GFX IP version 0x%x for gfx_v9_0\n",
+			amdgpu_ip_version(adev, GC_HWIP, 0));
+		return -EINVAL;
 	}
 
 	adev->gfx.config.gb_addr_config = gb_addr_config;
@@ -2808,7 +2810,8 @@ static void gfx_v9_1_parse_ind_reg_list(int *register_list_format,
 					break;
 			}
 
-			BUG_ON(idx >= unique_indirect_reg_count);
+			if (WARN_ON_ONCE(idx >= unique_indirect_reg_count))
+				break;
 
 			if (!unique_indirect_regs[idx])
 				unique_indirect_regs[idx] = register_list_format[indirect_offset];
@@ -2885,7 +2888,8 @@ static int gfx_v9_1_init_rlc_save_restore_list(struct amdgpu_device *adev)
 			}
 		}
 
-		BUG_ON(j >= unique_indirect_reg_count);
+		if (WARN_ON_ONCE(j >= unique_indirect_reg_count))
+			break;
 
 		i++;
 	}
@@ -4209,7 +4213,8 @@ static uint64_t gfx_v9_0_kiq_read_clock(struct amdgpu_device *adev)
 	struct amdgpu_kiq *kiq = &adev->gfx.kiq[0];
 	struct amdgpu_ring *ring = &kiq->ring;
 
-	BUG_ON(!ring->funcs->emit_rreg);
+	if (WARN_ON_ONCE(!ring->funcs->emit_rreg))
+		return 0;
 
 	spin_lock_irqsave(&kiq->ring_lock, flags);
 	if (amdgpu_device_wb_get(adev, &reg_val_offs)) {
@@ -5431,7 +5436,8 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
 	}
 
 	amdgpu_ring_write(ring, header);
-	BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
+	if (WARN_ON_ONCE(ib->gpu_addr & 0x3)) /* Dword align */
+		ib->gpu_addr &= ~0x3ULL;
 	amdgpu_ring_write(ring,
 #ifdef __BIG_ENDIAN
 		(2 << 0) |
@@ -5527,7 +5533,8 @@ static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
 	}
 
 	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
-	BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
+	if (WARN_ON_ONCE(ib->gpu_addr & 0x3)) /* Dword align */
+		ib->gpu_addr &= ~0x3ULL;
 	amdgpu_ring_write(ring,
 #ifdef __BIG_ENDIAN
 				(2 << 0) |
@@ -5567,10 +5574,13 @@ static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
 	 * the address should be Qword aligned if 64bit write, Dword
 	 * aligned if only send 32bit data low (discard data high)
 	 */
-	if (write64bit)
-		BUG_ON(addr & 0x7);
-	else
-		BUG_ON(addr & 0x3);
+	if (write64bit) {
+		if (WARN_ON_ONCE(addr & 0x7))
+			addr &= ~0x7ULL;
+	} else {
+		if (WARN_ON_ONCE(addr & 0x3))
+			addr &= ~0x3ULL;
+	}
 	amdgpu_ring_write(ring, lower_32_bits(addr));
 	amdgpu_ring_write(ring, upper_32_bits(addr));
 	amdgpu_ring_write(ring, lower_32_bits(seq));
@@ -5639,10 +5649,13 @@ static u64 gfx_v9_0_ring_get_wptr_compute(struct amdgpu_ring *ring)
 	u64 wptr;
 
 	/* XXX check if swapping is necessary on BE */
-	if (ring->use_doorbell)
+	if (ring->use_doorbell) {
 		wptr = atomic64_read((atomic64_t *)ring->wptr_cpu_addr);
-	else
-		BUG();
+	} else {
+		WARN_ONCE(1, "gfx_v9_0: non-doorbell wptr read on ring %s, only doorbell method supported on gfx9\n",
+			  ring->name);
+		wptr = 0;
+	}
 	return wptr;
 }
 
@@ -5654,8 +5667,9 @@ static void gfx_v9_0_ring_set_wptr_compute(struct amdgpu_ring *ring)
 	if (ring->use_doorbell) {
 		atomic64_set((atomic64_t *)ring->wptr_cpu_addr, ring->wptr);
 		WDOORBELL64(ring->doorbell_index, ring->wptr);
-	} else{
-		BUG(); /* only DOORBELL method supported on gfx9 now */
+	} else {
+		WARN_ONCE(1, "gfx_v9_0: non-doorbell wptr write on ring %s, only doorbell method supported on gfx9\n",
+			  ring->name);
 	}
 }
 
-- 
2.43.0

Re: [PATCH 2/4] drm/amdgpu/gfx9: replace BUG_ON/BUG with WARN_ON_ONCE in ring emission

[Christian König]

On 4/26/26 23:52, jbmoore wrote:
> From: "John B. Moore" <jbmoore61@gmail.com>
> 
> Replace all BUG_ON() and BUG() assertions in the gfx_v9_0 ring
> emission paths with WARN_ON_ONCE() and graceful recovery.  Ten sites
> are converted across wait_reg_mem, gpu_early_init, parse_ind_reg_list,
> init_rlc_save_restore_list, kiq_read_clock, emit_ib_gfx,
> emit_ib_compute, emit_fence, get_wptr_compute, and set_wptr_compute.
> 
> These assertions guard conditions that are either:
> - Address alignment checks on a deprecated byte-swap encoding from
>   legacy pre-amdgpu hardware (bits [1:0] must be zero), or
> - Switch-case defaults that should be unreachable but are better
>   handled with dev_err + return -EINVAL than a kernel panic.
> 
> Several of the address alignment BUG_ON sites in the IB emission
> paths (emit_ib_gfx, emit_ib_compute) are reachable from unprivileged
> userspace via crafted DRM_IOCTL_AMDGPU_CS submissions, causing a
> fatal kernel panic in a scheduler worker thread.
> 
> For address checks, clear the reserved bits and proceed.  For
> unreachable switch defaults, log the error and return.  For the
> doorbell-only wptr paths, log with WARN_ONCE and return zero /
> no-op.  Ring emission callbacks return void, so force-aligning
> and proceeding is the accepted pattern.
> 
> Found by a custom amdgpu DRM ioctl fuzzer.
> 
> Fixes: b1023571479020e9 ("drm/amdgpu: implement GFX 9.0 support (v2)")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 50 +++++++++++++++++----------
>  1 file changed, 32 insertions(+), 18 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 2eb32f92a..47e81c33d 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -1182,8 +1182,8 @@ static void gfx_v9_0_wait_reg_mem(struct amdgpu_ring *ring, int eng_sel,
>  				 WAIT_REG_MEM_FUNCTION(3) |  /* equal */
>  				 WAIT_REG_MEM_ENGINE(eng_sel)));
>  
> -	if (mem_space)
> -		BUG_ON(addr0 & 0x3); /* Dword align */
> +	if (mem_space && WARN_ON_ONCE(addr0 & 0x3))
> +		addr0 &= ~0x3; /* Force dword align */

Same comment as with the SDMA, please use only WARN_ON() and not WARN_ON_ONCE() and don't mask the bits.

>  	amdgpu_ring_write(ring, addr0);
>  	amdgpu_ring_write(ring, addr1);
>  	amdgpu_ring_write(ring, ref);
> @@ -2107,8 +2107,10 @@ static int gfx_v9_0_gpu_early_init(struct amdgpu_device *adev)
>  			return err;
>  		break;
>  	default:
> -		BUG();
> -		break;
> +		dev_err(adev->dev,
> +			"unsupported GFX IP version 0x%x for gfx_v9_0\n",
> +			amdgpu_ip_version(adev, GC_HWIP, 0));
> +		return -EINVAL;

Mhm, that is most likely a bad idea.

The BUG() here is perfectly justified because the system will crash later on anyway and this way we at least stop at the earliest possible time.

Regards,
Christian.

>  	}
>  
>  	adev->gfx.config.gb_addr_config = gb_addr_config;
> @@ -2808,7 +2810,8 @@ static void gfx_v9_1_parse_ind_reg_list(int *register_list_format,
>  					break;
>  			}
>  
> -			BUG_ON(idx >= unique_indirect_reg_count);
> +			if (WARN_ON_ONCE(idx >= unique_indirect_reg_count))
> +				break;
>  
>  			if (!unique_indirect_regs[idx])
>  				unique_indirect_regs[idx] = register_list_format[indirect_offset];
> @@ -2885,7 +2888,8 @@ static int gfx_v9_1_init_rlc_save_restore_list(struct amdgpu_device *adev)
>  			}
>  		}
>  
> -		BUG_ON(j >= unique_indirect_reg_count);
> +		if (WARN_ON_ONCE(j >= unique_indirect_reg_count))
> +			break;
>  
>  		i++;
>  	}
> @@ -4209,7 +4213,8 @@ static uint64_t gfx_v9_0_kiq_read_clock(struct amdgpu_device *adev)
>  	struct amdgpu_kiq *kiq = &adev->gfx.kiq[0];
>  	struct amdgpu_ring *ring = &kiq->ring;
>  
> -	BUG_ON(!ring->funcs->emit_rreg);
> +	if (WARN_ON_ONCE(!ring->funcs->emit_rreg))
> +		return 0;
>  
>  	spin_lock_irqsave(&kiq->ring_lock, flags);
>  	if (amdgpu_device_wb_get(adev, &reg_val_offs)) {
> @@ -5431,7 +5436,8 @@ static void gfx_v9_0_ring_emit_ib_gfx(struct amdgpu_ring *ring,
>  	}
>  
>  	amdgpu_ring_write(ring, header);
> -	BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> +	if (WARN_ON_ONCE(ib->gpu_addr & 0x3)) /* Dword align */
> +		ib->gpu_addr &= ~0x3ULL;
>  	amdgpu_ring_write(ring,
>  #ifdef __BIG_ENDIAN
>  		(2 << 0) |
> @@ -5527,7 +5533,8 @@ static void gfx_v9_0_ring_emit_ib_compute(struct amdgpu_ring *ring,
>  	}
>  
>  	amdgpu_ring_write(ring, PACKET3(PACKET3_INDIRECT_BUFFER, 2));
> -	BUG_ON(ib->gpu_addr & 0x3); /* Dword align */
> +	if (WARN_ON_ONCE(ib->gpu_addr & 0x3)) /* Dword align */
> +		ib->gpu_addr &= ~0x3ULL;
>  	amdgpu_ring_write(ring,
>  #ifdef __BIG_ENDIAN
>  				(2 << 0) |
> @@ -5567,10 +5574,13 @@ static void gfx_v9_0_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>  	 * the address should be Qword aligned if 64bit write, Dword
>  	 * aligned if only send 32bit data low (discard data high)
>  	 */
> -	if (write64bit)
> -		BUG_ON(addr & 0x7);
> -	else
> -		BUG_ON(addr & 0x3);
> +	if (write64bit) {
> +		if (WARN_ON_ONCE(addr & 0x7))
> +			addr &= ~0x7ULL;
> +	} else {
> +		if (WARN_ON_ONCE(addr & 0x3))
> +			addr &= ~0x3ULL;
> +	}
>  	amdgpu_ring_write(ring, lower_32_bits(addr));
>  	amdgpu_ring_write(ring, upper_32_bits(addr));
>  	amdgpu_ring_write(ring, lower_32_bits(seq));
> @@ -5639,10 +5649,13 @@ static u64 gfx_v9_0_ring_get_wptr_compute(struct amdgpu_ring *ring)
>  	u64 wptr;
>  
>  	/* XXX check if swapping is necessary on BE */
> -	if (ring->use_doorbell)
> +	if (ring->use_doorbell) {
>  		wptr = atomic64_read((atomic64_t *)ring->wptr_cpu_addr);
> -	else
> -		BUG();
> +	} else {
> +		WARN_ONCE(1, "gfx_v9_0: non-doorbell wptr read on ring %s, only doorbell method supported on gfx9\n",
> +			  ring->name);
> +		wptr = 0;
> +	}
>  	return wptr;
>  }
>  
> @@ -5654,8 +5667,9 @@ static void gfx_v9_0_ring_set_wptr_compute(struct amdgpu_ring *ring)
>  	if (ring->use_doorbell) {
>  		atomic64_set((atomic64_t *)ring->wptr_cpu_addr, ring->wptr);
>  		WDOORBELL64(ring->doorbell_index, ring->wptr);
> -	} else{
> -		BUG(); /* only DOORBELL method supported on gfx9 now */
> +	} else {
> +		WARN_ONCE(1, "gfx_v9_0: non-doorbell wptr write on ring %s, only doorbell method supported on gfx9\n",
> +			  ring->name);
>  	}
>  }
>  

[PATCH 3/4] drm/amdgpu/gfx9: replace BUG_ON with WARN_ON_ONCE for KIQ 64-bit fence flag

[jbmoore]

From: "John B. Moore" <jbmoore61@gmail.com>

gfx_v9_0_ring_emit_fence_kiq() contains a BUG_ON() that fires when
the AMDGPU_FENCE_FLAG_64BIT flag is passed.  The KIQ (Kernel
Interface Queue) ring only allocates 32-bit writeback buffer
addresses for fence sequence numbers.  A 64-bit fence write would
overflow the allocated writeback slot, potentially corrupting
adjacent kernel memory.

Replace BUG_ON() with WARN_ON_ONCE() and mask off the unsupported
flag.  This prevents the kernel panic while still logging the
unexpected condition and falling back to a safe 32-bit fence write.

This is separated from the main gfx9 BUG_ON conversion patch
because it addresses a different security concern (potential buffer
overflow in kernel-managed writeback memory) rather than the address
alignment assertions in the ring emission paths.

Found by a custom amdgpu DRM ioctl fuzzer.

Fixes: b1023571479020e9 ("drm/amdgpu: implement GFX 9.0 support (v2)")
Signed-off-by: John B. Moore <jbmoore61@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
index 47e81c33d..fb2a0f1af 100644
--- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
@@ -5679,7 +5679,8 @@ static void gfx_v9_0_ring_emit_fence_kiq(struct amdgpu_ring *ring, u64 addr,
 	struct amdgpu_device *adev = ring->adev;
 
 	/* we only allocate 32bit for each seq wb address */
-	BUG_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
+	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
+		flags &= ~AMDGPU_FENCE_FLAG_64BIT;
 
 	/* write fence seq to the "addr" */
 	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));
-- 
2.43.0

Re: [PATCH 3/4] drm/amdgpu/gfx9: replace BUG_ON with WARN_ON_ONCE for KIQ 64-bit fence flag

[Christian König]

On 4/26/26 23:52, jbmoore wrote:
> From: "John B. Moore" <jbmoore61@gmail.com>
> 
> gfx_v9_0_ring_emit_fence_kiq() contains a BUG_ON() that fires when
> the AMDGPU_FENCE_FLAG_64BIT flag is passed.  The KIQ (Kernel
> Interface Queue) ring only allocates 32-bit writeback buffer
> addresses for fence sequence numbers.  A 64-bit fence write would
> overflow the allocated writeback slot, potentially corrupting
> adjacent kernel memory.
> 
> Replace BUG_ON() with WARN_ON_ONCE() and mask off the unsupported
> flag.  This prevents the kernel panic while still logging the
> unexpected condition and falling back to a safe 32-bit fence write.
> 
> This is separated from the main gfx9 BUG_ON conversion patch
> because it addresses a different security concern (potential buffer
> overflow in kernel-managed writeback memory) rather than the address
> alignment assertions in the ring emission paths.
> 
> Found by a custom amdgpu DRM ioctl fuzzer.
> 
> Fixes: b1023571479020e9 ("drm/amdgpu: implement GFX 9.0 support (v2)")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 47e81c33d..fb2a0f1af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -5679,7 +5679,8 @@ static void gfx_v9_0_ring_emit_fence_kiq(struct amdgpu_ring *ring, u64 addr,
>  	struct amdgpu_device *adev = ring->adev;
>  
>  	/* we only allocate 32bit for each seq wb address */
> -	BUG_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))

Please use only WARN_ON() and not WARN_ON_ONCE().

> +		flags &= ~AMDGPU_FENCE_FLAG_64BIT;

I don't think we should mask the flag here.

Regards,
Christian.

>  
>  	/* write fence seq to the "addr" */
>  	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));

Re: [PATCH 3/4] drm/amdgpu/gfx9: replace BUG_ON with WARN_ON_ONCE for KIQ 64-bit fence flag

[Christian König]

On 4/26/26 23:52, jbmoore wrote:
> From: "John B. Moore" <jbmoore61@gmail.com>
> 
> gfx_v9_0_ring_emit_fence_kiq() contains a BUG_ON() that fires when
> the AMDGPU_FENCE_FLAG_64BIT flag is passed.  The KIQ (Kernel
> Interface Queue) ring only allocates 32-bit writeback buffer
> addresses for fence sequence numbers.  A 64-bit fence write would
> overflow the allocated writeback slot, potentially corrupting
> adjacent kernel memory.
> 
> Replace BUG_ON() with WARN_ON_ONCE() and mask off the unsupported
> flag.  This prevents the kernel panic while still logging the
> unexpected condition and falling back to a safe 32-bit fence write.
> 
> This is separated from the main gfx9 BUG_ON conversion patch
> because it addresses a different security concern (potential buffer
> overflow in kernel-managed writeback memory) rather than the address
> alignment assertions in the ring emission paths.
> 
> Found by a custom amdgpu DRM ioctl fuzzer.
> 
> Fixes: b1023571479020e9 ("drm/amdgpu: implement GFX 9.0 support (v2)")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org

And completely forgotten: Please drop the CC stable here. That is unjustified for this patch.

Regards,
Christian.

> ---
>  drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> index 47e81c33d..fb2a0f1af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/gfx_v9_0.c
> @@ -5679,7 +5679,8 @@ static void gfx_v9_0_ring_emit_fence_kiq(struct amdgpu_ring *ring, u64 addr,
>  	struct amdgpu_device *adev = ring->adev;
>  
>  	/* we only allocate 32bit for each seq wb address */
> -	BUG_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		flags &= ~AMDGPU_FENCE_FLAG_64BIT;
>  
>  	/* write fence seq to the "addr" */
>  	amdgpu_ring_write(ring, PACKET3(PACKET3_WRITE_DATA, 3));

[PATCH 4/4] drm/amdgpu/vcn: prevent silent fence drop on 64-bit flag mismatch

[jbmoore]

From: "John B. Moore" <jbmoore61@gmail.com>

VCN, UVD, and VCE encoder/decoder ring fence emission callbacks only
support 32-bit fence writes.  When AMDGPU_FENCE_FLAG_64BIT is passed,
the existing bare WARN_ON() fires but execution continues, emitting
a truncated fence that causes the VCN hardware unit to issue a
no-retry UTCL2 page fault at NULL address (0x0).

The hardware fault is non-recoverable: the VCNU client is permanently
stalled, the VCN ring stops processing jobs, and all pending fences
on the affected ring never signal.

Convert WARN_ON() to WARN_ON_ONCE() and add an early return to
prevent the corrupted fence emission.  The early return is safe
because the WARN_ON fires before any ring buffer writes in all five
affected callsites:
  - vcn_v1_0_dec_ring_emit_fence()
  - vcn_v1_0_enc_ring_emit_fence()
  - vcn_v2_0_dec_ring_emit_fence()
  - vcn_v2_0_enc_ring_emit_fence()
  - vcn_dec_sw_ring_emit_fence()

The missing fence will be caught by the scheduler timeout mechanism,
which will clean up the job without hardware damage.

Using WARN_ON_ONCE instead of the bare WARN_ON also prevents kernel
log flooding if the condition is triggered repeatedly by a fuzzer.

Found by a custom amdgpu DRM ioctl fuzzer.

Fixes: 8ace845ff0e8 ("drm/amdgpu: add vcn enc ring type and functions")
Fixes: cca69fe8ff98 ("drm/amdgpu: add vcn decode ring type and functions")
Signed-off-by: John B. Moore <jbmoore61@gmail.com>
Cc: stable@vger.kernel.org
---
 drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c | 3 ++-
 drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 6 ++++--
 drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c    | 6 ++++--
 3 files changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
index 2b9ddb3d2..aa0022deb 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
@@ -27,7 +27,8 @@
 void vcn_dec_sw_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
 	u64 seq, uint32_t flags)
 {
-	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
+	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
+		return;
 
 	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_FENCE);
 	amdgpu_ring_write(ring, addr);
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
index e9d790914..2acf6e621 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
@@ -1548,7 +1548,8 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
 {
 	struct amdgpu_device *adev = ring->adev;
 
-	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
+	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
+		return;
 
 	amdgpu_ring_write(ring,
 		PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_CONTEXT_ID), 0));
@@ -1724,7 +1725,8 @@ static void vcn_v1_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
 static void vcn_v1_0_enc_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
 			u64 seq, unsigned flags)
 {
-	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
+	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
+		return;
 
 	amdgpu_ring_write(ring, VCN_ENC_CMD_FENCE);
 	amdgpu_ring_write(ring, addr);
diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
index e35fae9cd..6cfb5aedd 100644
--- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
+++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
@@ -1537,7 +1537,8 @@ void vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
 {
 	struct amdgpu_device *adev = ring->adev;
 
-	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
+	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
+		return;
 	amdgpu_ring_write(ring, PACKET0(adev->vcn.inst[ring->me].internal.context_id, 0));
 	amdgpu_ring_write(ring, seq);
 
@@ -1722,7 +1723,8 @@ static void vcn_v2_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
 void vcn_v2_0_enc_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
 				u64 seq, unsigned flags)
 {
-	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
+	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
+		return;
 
 	amdgpu_ring_write(ring, VCN_ENC_CMD_FENCE);
 	amdgpu_ring_write(ring, addr);
-- 
2.43.0

Re: [PATCH 4/4] drm/amdgpu/vcn: prevent silent fence drop on 64-bit flag mismatch

[Christian König]

On 4/26/26 23:52, jbmoore wrote:
> From: "John B. Moore" <jbmoore61@gmail.com>
> 
> VCN, UVD, and VCE encoder/decoder ring fence emission callbacks only
> support 32-bit fence writes.  When AMDGPU_FENCE_FLAG_64BIT is passed,
> the existing bare WARN_ON() fires but execution continues, emitting
> a truncated fence that causes the VCN hardware unit to issue a
> no-retry UTCL2 page fault at NULL address (0x0).
> 
> The hardware fault is non-recoverable: the VCNU client is permanently
> stalled, the VCN ring stops processing jobs, and all pending fences
> on the affected ring never signal.
> 
> Convert WARN_ON() to WARN_ON_ONCE() and add an early return to
> prevent the corrupted fence emission.  The early return is safe
> because the WARN_ON fires before any ring buffer writes in all five
> affected callsites:
>   - vcn_v1_0_dec_ring_emit_fence()
>   - vcn_v1_0_enc_ring_emit_fence()
>   - vcn_v2_0_dec_ring_emit_fence()
>   - vcn_v2_0_enc_ring_emit_fence()
>   - vcn_dec_sw_ring_emit_fence()
> 
> The missing fence will be caught by the scheduler timeout mechanism,
> which will clean up the job without hardware damage.
> 
> Using WARN_ON_ONCE instead of the bare WARN_ON also prevents kernel
> log flooding if the condition is triggered repeatedly by a fuzzer.
> 
> Found by a custom amdgpu DRM ioctl fuzzer.

Absolutely clear NAK. Not emitting the fence is even worse than the page fault.

Question is rather why that isn't filtered upfront by the CS IOCTL?

Regards,
Christian.

> 
> Fixes: 8ace845ff0e8 ("drm/amdgpu: add vcn enc ring type and functions")
> Fixes: cca69fe8ff98 ("drm/amdgpu: add vcn decode ring type and functions")
> Signed-off-by: John B. Moore <jbmoore61@gmail.com>
> Cc: stable@vger.kernel.org
> ---
>  drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c | 3 ++-
>  drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c    | 6 ++++--
>  drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c    | 6 ++++--
>  3 files changed, 10 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> index 2b9ddb3d2..aa0022deb 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_sw_ring.c
> @@ -27,7 +27,8 @@
>  void vcn_dec_sw_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>  	u64 seq, uint32_t flags)
>  {
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	amdgpu_ring_write(ring, VCN_DEC_SW_CMD_FENCE);
>  	amdgpu_ring_write(ring, addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> index e9d790914..2acf6e621 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v1_0.c
> @@ -1548,7 +1548,8 @@ static void vcn_v1_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64
>  {
>  	struct amdgpu_device *adev = ring->adev;
>  
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	amdgpu_ring_write(ring,
>  		PACKET0(SOC15_REG_OFFSET(UVD, 0, mmUVD_CONTEXT_ID), 0));
> @@ -1724,7 +1725,8 @@ static void vcn_v1_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
>  static void vcn_v1_0_enc_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>  			u64 seq, unsigned flags)
>  {
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	amdgpu_ring_write(ring, VCN_ENC_CMD_FENCE);
>  	amdgpu_ring_write(ring, addr);
> diff --git a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> index e35fae9cd..6cfb5aedd 100644
> --- a/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> +++ b/drivers/gpu/drm/amd/amdgpu/vcn_v2_0.c
> @@ -1537,7 +1537,8 @@ void vcn_v2_0_dec_ring_emit_fence(struct amdgpu_ring *ring, u64 addr, u64 seq,
>  {
>  	struct amdgpu_device *adev = ring->adev;
>  
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  	amdgpu_ring_write(ring, PACKET0(adev->vcn.inst[ring->me].internal.context_id, 0));
>  	amdgpu_ring_write(ring, seq);
>  
> @@ -1722,7 +1723,8 @@ static void vcn_v2_0_enc_ring_set_wptr(struct amdgpu_ring *ring)
>  void vcn_v2_0_enc_ring_emit_fence(struct amdgpu_ring *ring, u64 addr,
>  				u64 seq, unsigned flags)
>  {
> -	WARN_ON(flags & AMDGPU_FENCE_FLAG_64BIT);
> +	if (WARN_ON_ONCE(flags & AMDGPU_FENCE_FLAG_64BIT))
> +		return;
>  
>  	amdgpu_ring_write(ring, VCN_ENC_CMD_FENCE);
>  	amdgpu_ring_write(ring, addr);