On 23/06/2025 18.06, Yan Zhai wrote:
We encountered following crash when testing a XDP_REDIRECT feature
in production:
[...]
From the crash dump, we found that the cpu_map_flush_list inside
redirect info is partially corrupted: its list_head->next points to
itself, but list_head->prev points to a valid list of unflushed bq
entries.
This turned out to be a result of missed XDP flush on redirect lists. By
digging in the actual source code, we found that
commit 7f0a168b0441 ("bnxt_en: Add completion ring pointer in TX and RX
ring structures") incorrectly overwrites the event mask for XDP_REDIRECT
in bnxt_rx_xdp.
(To Andy + Michael:)
The initial bug was introduced in [1] commit a7559bc8c17c ("bnxt:
support transmit and free of aggregation buffers") in bnxt_rx_xdp()
where case XDP_TX zeros the *event, that also carries the XDP-redirect
indication.
I'm wondering if the driver should not reset the *event value?
(all other drive code paths doesn't)
We can stably reproduce this crash by returning XDP_TX
and XDP_REDIRECT randomly for incoming packets in a naive XDP program.
Properly propagate the XDP_REDIRECT events back fixes the crash.
Fixes: 7f0a168b0441 ("bnxt_en: Add completion ring pointer in TX and RX ring structures")
We should also add:
Fixes: a7559bc8c17c ("bnxt: support transmit and free of aggregation
buffers")
[0] https://git.kernel.org/torvalds/c/7f0a168b0441 - v6.8-rc1
[1] https://git.kernel.org/torvalds/c/a7559bc8c17c - v5.19-rc1
Tested-by: Andrew Rzeznik <arzeznik@xxxxxxxxxxxxxx>
Signed-off-by: Yan Zhai <yan@xxxxxxxxxxxxxx>
Acked-by: Jesper Dangaard Brouer <hawk@xxxxxxxxxx>
---
drivers/net/ethernet/broadcom/bnxt/bnxt.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
index 2cb3185c442c..ae89a981e052 100644
--- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
+++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
@@ -2989,6 +2989,7 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
{
struct bnxt_napi *bnapi = cpr->bnapi;
u32 raw_cons = cpr->cp_raw_cons;
+ bool flush_xdp = false;
u32 cons;
int rx_pkts = 0;
u8 event = 0;
@@ -3042,6 +3043,8 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
else
rc = bnxt_force_rx_discard(bp, cpr, &raw_cons,
&event);
+ if (event & BNXT_REDIRECT_EVENT)
+ flush_xdp = true;
if (likely(rc >= 0))
rx_pkts += rc;
/* Increment rx_pkts when rc is -ENOMEM to count towards
@@ -3066,7 +3069,7 @@ static int __bnxt_poll_work(struct bnxt *bp, struct bnxt_cp_ring_info *cpr,
}
}
- if (event & BNXT_REDIRECT_EVENT) {
+ if (flush_xdp) {
xdp_do_flush();
event &= ~BNXT_REDIRECT_EVENT;
}
