Re: NULL pointer deref inside xdp_do_flush due to bpf_net_ctx_get_all_used_flush_lists

[Sebastian Andrzej Siewior <bigeasy@linutronix.de>]

On 2024-10-01 13:30:13 [+0200], Yury Vostrikov wrote:
> Hi,
Hi,

…
> I get the following backtrace instead of crash:
> 
> [  177.216427] ------------[ cut here ]------------
…
> [  177.216464] Call Trace:
> [  177.216464]  <TASK>
> [  177.216474]  efx_poll+0x178/0x380 [sfc_siena]
> [  177.216479]  netpoll_poll_dev+0x118/0x1b0
> [  177.216481]  __netpoll_send_skb+0x1ae/0x240
> [  177.216482]  netpoll_send_udp+0x2e5/0x400
> [  177.216484]  write_msg+0xeb/0x100 [netconsole]
> [  177.216486]  console_flush_all+0x261/0x440
> [  177.216489]  console_unlock+0x71/0xf0
> [  177.216490]  vprintk_emit+0x251/0x2b0
> [  177.216491]  _printk+0x48/0x50
…
> I'm out of my depth figuring out why bpf_net_ctx_get() returns NULL. For now I'm simply running with NULL check enabled.

netpoll_send_udp() Does not assign a context and invokes a NAPI poll.
However with a budget of 0 to just clean the TX resources.
Now, the SFC driver does not clean any RX packets but invokes
xdp_do_flush() anyway which leads to the crash later on.
Are the SFC maintainer against the following:

diff --git a/drivers/net/ethernet/sfc/efx_channels.c b/drivers/net/ethernet/sfc/efx_channels.c
index c9e17a8208a90..f3288e02c1bd8 100644
--- a/drivers/net/ethernet/sfc/efx_channels.c
+++ b/drivers/net/ethernet/sfc/efx_channels.c
@@ -1260,7 +1260,8 @@ static int efx_poll(struct napi_struct *napi, int budget)
 
 	spent = efx_process_channel(channel, budget);
 
-	xdp_do_flush();
+	if (spent)
+		xdp_do_flush();
 
 	if (spent < budget) {
 		if (efx_channel_has_rx_queue(channel) &&
diff --git a/drivers/net/ethernet/sfc/siena/efx_channels.c b/drivers/net/ethernet/sfc/siena/efx_channels.c
index a7346e965bfe7..2b8b7c69bd7ae 100644
--- a/drivers/net/ethernet/sfc/siena/efx_channels.c
+++ b/drivers/net/ethernet/sfc/siena/efx_channels.c
@@ -1285,7 +1285,8 @@ static int efx_poll(struct napi_struct *napi, int budget)
 
 	spent = efx_process_channel(channel, budget);
 
-	xdp_do_flush();
+	if (spent)
+		xdp_do_flush();
 
 	if (spent < budget) {
 		if (efx_channel_has_rx_queue(channel) &&

This should fix the crash. As an alternative we could keep track of
channel->n_rx_xdp_redirect before and after the efx_process_channel()
invocation to avoid the flush if there is no XDP done.

Sebastian