netdev - Re: [PATCH net] bnxt: properly flush XDP redirect lists

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <633986ae-75c4-44fa-96f8-2dde00e17530@kernel.org>
Date: Tue, 24 Jun 2025 07:59:26 +0200
From: Jesper Dangaard Brouer <hawk@...nel.org>
To: Yan Zhai <yan@...udflare.com>, netdev@...r.kernel.org
Cc: Michael Chan <michael.chan@...adcom.com>,
 Pavan Chebbi <pavan.chebbi@...adcom.com>, Andrew Lunn
 <andrew+netdev@...n.ch>, "David S. Miller" <davem@...emloft.net>,
 Eric Dumazet <edumazet@...gle.com>, Jakub Kicinski <kuba@...nel.org>,
 Paolo Abeni <pabeni@...hat.com>, Alexei Starovoitov <ast@...nel.org>,
 Daniel Borkmann <daniel@...earbox.net>,
 John Fastabend <john.fastabend@...il.com>,
 Stanislav Fomichev <sdf@...ichev.me>,
 Andy Gospodarek <andrew.gospodarek@...adcom.com>,
 linux-kernel@...r.kernel.org, bpf@...r.kernel.org, kernel-team@...udflare.com
Subject: Re: [PATCH net] bnxt: properly flush XDP redirect lists

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@...udflare.com>
> Signed-off-by: Yan Zhai <yan@...udflare.com>

Acked-by: Jesper Dangaard Brouer <hawk@...nel.org>

> ---
>   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;
>   	}