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.