Thread (14 messages) 14 messages, 4 authors, 4d ago

Re: [PATCH net v3 5/5] eth: fbnic: bound the XDP Tx completion head to the ring

flat view

From: Mohsin Bashir <hidden>
Date: 2026-10-12 08:34:31
Also in: lkml, stable


On 10/11/26 9:16 PM, Yehyeong Lee wrote:
quoted hunk ↗ jump to hunk
fbnic_clean_twq1() walks tx_buf[] from ring->head to the head the device
reports in the Tx completion (FBNIC_TCD_TYPE0_HEAD1_MASK, a 16-bit
field). head is masked to the ring, the device head is not, so a head
outside the 1024-entry ring is never reached and the loop does not
terminate; it keeps releasing the same buffers to the page pool. A head
that is inside the ring but past the posted tail walks over descriptors
from an earlier wrap and releases their pages a second time.

Return without cleaning if the device head is outside the ring or outside
the posted window, the way fbnic_clean_bdq() already guards its head.
fbnic_clean_twq0() masks the head difference to the ring and is finite;
twq1 did not, which this brings in line. A head equal to the tail is
accepted, as fbnic_nv_flush() passes the tail to drain the whole ring.

   Oops: general protection fault, probably for non-canonical address
   0xfbd59c000000002b: 0000 [#1] SMP KASAN NOPTI
   KASAN: maybe wild-memory-access in range
   [0xdead000000000158-0xdead00000000015f]
   RIP: 0010:page_pool_return_netmem+0x2a/0x3f0
   Call Trace:
    <TASK>
    fbnic_clean_twq1.isra.0+0x20c/0x410 [fbnic]

The loop was exercised by passing a crafted completion head to
fbnic_clean_twq1() under KASAN; no hardware is involved. On a kernel
without soft-lockup detection the repeated release trips a page_pool
use-after-free before the loop is otherwise noticed.

Fixes: cf4facfb132a ("eth: fbnic: Add support for XDP queues")
Cc: stable@vger.kernel.org
Signed-off-by: Yehyeong Lee <redacted>
---
  drivers/net/ethernet/meta/fbnic/fbnic_txrx.c | 6 ++++++
  1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
index 7a9bd91bcc41c..3a148eb4e560f 100644
--- a/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
+++ b/drivers/net/ethernet/meta/fbnic/fbnic_txrx.c
@@ -661,6 +661,12 @@ static void fbnic_clean_twq1(struct fbnic_napi_vector *nv, bool pp_allow_direct,
  	u64 total_bytes = 0, total_packets = 0;
  	unsigned int head = ring->head;
  
+	/* bound the device head to the ring; see fbnic_clean_twq0() */
+	if (unlikely(hw_head > ring->size_mask ||
+		     ((hw_head - head) & ring->size_mask) >
+		     fbnic_desc_used(ring)))
+		return;
+
  	while (hw_head != head) {
  		struct page *page;
  		u64 twd;
The device can only consume descriptors up to the tail published by the driver. A completion head outside the ring violates that contract which can be hardening but not a bug fix. IMHO such a defensive hardening may not be a good idea.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help