| Message ID | 20240620105808.3496993-1-ap420073@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | e3f02f32a05009a688a87f5799e049ed6b55bab5 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v2] ionic: fix kernel panic due to multi-buffer handling | expand |
On 6/20/2024 3:58 AM, Taehee Yoo wrote: > Caution: This message originated from an External Source. Use proper caution when opening attachments, clicking links, or responding. > > > Currently, the ionic_run_xdp() doesn't handle multi-buffer packets > properly for XDP_TX and XDP_REDIRECT. > When a jumbo frame is received, the ionic_run_xdp() first makes xdp > frame with all necessary pages in the rx descriptor. > And if the action is either XDP_TX or XDP_REDIRECT, it should unmap > dma-mapping and reset page pointer to NULL for all pages, not only the > first page. > But it doesn't for SG pages. So, SG pages unexpectedly will be reused. > It eventually causes kernel panic. > > Oops: general protection fault, probably for non-canonical address 0x504f4e4dbebc64ff: 0000 [#1] PREEMPT SMP NOPTI > CPU: 3 PID: 0 Comm: swapper/3 Not tainted 6.10.0-rc3+ #25 > RIP: 0010:xdp_return_frame+0x42/0x90 > Code: 01 75 12 5b 4c 89 e6 5d 31 c9 41 5c 31 d2 41 5d e9 73 fd ff ff 44 8b 6b 20 0f b7 43 0a 49 81 ed 68 01 00 00 49 29 c5 49 01 fd <41> 80 7d0 > RSP: 0018:ffff99d00122ce08 EFLAGS: 00010202 > RAX: 0000000000005453 RBX: ffff8d325f904000 RCX: 0000000000000001 > RDX: 00000000670e1000 RSI: 000000011f90d000 RDI: 504f4e4d4c4b4a49 > RBP: ffff99d003907740 R08: 0000000000000000 R09: 0000000000000000 > R10: 000000011f90d000 R11: 0000000000000000 R12: ffff8d325f904010 > R13: 504f4e4dbebc64fd R14: ffff8d3242b070c8 R15: ffff99d0039077c0 > FS: 0000000000000000(0000) GS:ffff8d399f780000(0000) knlGS:0000000000000000 > CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 > CR2: 00007f41f6c85e38 CR3: 000000037ac30000 CR4: 00000000007506f0 > PKRU: 55555554 > Call Trace: > <IRQ> > ? die_addr+0x33/0x90 > ? exc_general_protection+0x251/0x2f0 > ? asm_exc_general_protection+0x22/0x30 > ? xdp_return_frame+0x42/0x90 > ionic_tx_clean+0x211/0x280 [ionic 15881354510e6a9c655c59c54812b319ed2cd015] > ionic_tx_cq_service+0xd3/0x210 [ionic 15881354510e6a9c655c59c54812b319ed2cd015] > ionic_txrx_napi+0x41/0x1b0 [ionic 15881354510e6a9c655c59c54812b319ed2cd015] > __napi_poll.constprop.0+0x29/0x1b0 > net_rx_action+0x2c4/0x350 > handle_softirqs+0xf4/0x320 > irq_exit_rcu+0x78/0xa0 > common_interrupt+0x77/0x90 > > Fixes: 5377805dc1c0 ("ionic: implement xdp frags support") > Signed-off-by: Taehee Yoo <ap420073@gmail.com> > --- > > v2: > - Use ionic_xdp_rx_put_bufs() instead of open code. > > .../net/ethernet/pensando/ionic/ionic_txrx.c | 27 ++++++++++++------- > 1 file changed, 18 insertions(+), 9 deletions(-) > > diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > index 2427610f4306..aed7d9cbce03 100644 > --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c > @@ -480,6 +480,20 @@ int ionic_xdp_xmit(struct net_device *netdev, int n, > return nxmit; > } > > +static void ionic_xdp_rx_put_bufs(struct ionic_queue *q, > + struct ionic_buf_info *buf_info, > + int nbufs) > +{ > + int i; > + > + for (i = 0; i < nbufs; i++) { > + dma_unmap_page(q->dev, buf_info->dma_addr, > + IONIC_PAGE_SIZE, DMA_FROM_DEVICE); > + buf_info->page = NULL; > + buf_info++; > + } > +} > + > static bool ionic_run_xdp(struct ionic_rx_stats *stats, > struct net_device *netdev, > struct bpf_prog *xdp_prog, > @@ -493,6 +507,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, > struct netdev_queue *nq; > struct xdp_frame *xdpf; > int remain_len; > + int nbufs = 1; > int frag_len; > int err = 0; > > @@ -542,6 +557,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, > if (page_is_pfmemalloc(bi->page)) > xdp_buff_set_frag_pfmemalloc(&xdp_buf); > } while (remain_len > 0); > + nbufs += sinfo->nr_frags; > } > > xdp_action = bpf_prog_run_xdp(xdp_prog, &xdp_buf); > @@ -574,9 +590,6 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, > goto out_xdp_abort; > } > > - dma_unmap_page(rxq->dev, buf_info->dma_addr, > - IONIC_PAGE_SIZE, DMA_FROM_DEVICE); > - > err = ionic_xdp_post_frame(txq, xdpf, XDP_TX, > buf_info->page, > buf_info->page_offset, > @@ -586,23 +599,19 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, > netdev_dbg(netdev, "tx ionic_xdp_post_frame err %d\n", err); > goto out_xdp_abort; > } > - buf_info->page = NULL; > + ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs); > stats->xdp_tx++; > > /* the Tx completion will free the buffers */ > break; > > case XDP_REDIRECT: > - /* unmap the pages before handing them to a different device */ > - dma_unmap_page(rxq->dev, buf_info->dma_addr, > - IONIC_PAGE_SIZE, DMA_FROM_DEVICE); > - > err = xdp_do_redirect(netdev, &xdp_buf, xdp_prog); > if (err) { > netdev_dbg(netdev, "xdp_do_redirect err %d\n", err); > goto out_xdp_abort; > } > - buf_info->page = NULL; > + ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs); > rxq->xdp_flush = true; > stats->xdp_redirect++; > break; I think we can live with that. Thanks -sln Reviewed-by: Shannon Nelson <shannon.nelson@amd.com> > -- > 2.34.1 >
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Thu, 20 Jun 2024 10:58:08 +0000 you wrote: > Currently, the ionic_run_xdp() doesn't handle multi-buffer packets > properly for XDP_TX and XDP_REDIRECT. > When a jumbo frame is received, the ionic_run_xdp() first makes xdp > frame with all necessary pages in the rx descriptor. > And if the action is either XDP_TX or XDP_REDIRECT, it should unmap > dma-mapping and reset page pointer to NULL for all pages, not only the > first page. > But it doesn't for SG pages. So, SG pages unexpectedly will be reused. > It eventually causes kernel panic. > > [...] Here is the summary with links: - [net,v2] ionic: fix kernel panic due to multi-buffer handling https://git.kernel.org/netdev/net/c/e3f02f32a050 You are awesome, thank you!
diff --git a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c index 2427610f4306..aed7d9cbce03 100644 --- a/drivers/net/ethernet/pensando/ionic/ionic_txrx.c +++ b/drivers/net/ethernet/pensando/ionic/ionic_txrx.c @@ -480,6 +480,20 @@ int ionic_xdp_xmit(struct net_device *netdev, int n, return nxmit; } +static void ionic_xdp_rx_put_bufs(struct ionic_queue *q, + struct ionic_buf_info *buf_info, + int nbufs) +{ + int i; + + for (i = 0; i < nbufs; i++) { + dma_unmap_page(q->dev, buf_info->dma_addr, + IONIC_PAGE_SIZE, DMA_FROM_DEVICE); + buf_info->page = NULL; + buf_info++; + } +} + static bool ionic_run_xdp(struct ionic_rx_stats *stats, struct net_device *netdev, struct bpf_prog *xdp_prog, @@ -493,6 +507,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, struct netdev_queue *nq; struct xdp_frame *xdpf; int remain_len; + int nbufs = 1; int frag_len; int err = 0; @@ -542,6 +557,7 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, if (page_is_pfmemalloc(bi->page)) xdp_buff_set_frag_pfmemalloc(&xdp_buf); } while (remain_len > 0); + nbufs += sinfo->nr_frags; } xdp_action = bpf_prog_run_xdp(xdp_prog, &xdp_buf); @@ -574,9 +590,6 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, goto out_xdp_abort; } - dma_unmap_page(rxq->dev, buf_info->dma_addr, - IONIC_PAGE_SIZE, DMA_FROM_DEVICE); - err = ionic_xdp_post_frame(txq, xdpf, XDP_TX, buf_info->page, buf_info->page_offset, @@ -586,23 +599,19 @@ static bool ionic_run_xdp(struct ionic_rx_stats *stats, netdev_dbg(netdev, "tx ionic_xdp_post_frame err %d\n", err); goto out_xdp_abort; } - buf_info->page = NULL; + ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs); stats->xdp_tx++; /* the Tx completion will free the buffers */ break; case XDP_REDIRECT: - /* unmap the pages before handing them to a different device */ - dma_unmap_page(rxq->dev, buf_info->dma_addr, - IONIC_PAGE_SIZE, DMA_FROM_DEVICE); - err = xdp_do_redirect(netdev, &xdp_buf, xdp_prog); if (err) { netdev_dbg(netdev, "xdp_do_redirect err %d\n", err); goto out_xdp_abort; } - buf_info->page = NULL; + ionic_xdp_rx_put_bufs(rxq, buf_info, nbufs); rxq->xdp_flush = true; stats->xdp_redirect++; break;
Currently, the ionic_run_xdp() doesn't handle multi-buffer packets properly for XDP_TX and XDP_REDIRECT. When a jumbo frame is received, the ionic_run_xdp() first makes xdp frame with all necessary pages in the rx descriptor. And if the action is either XDP_TX or XDP_REDIRECT, it should unmap dma-mapping and reset page pointer to NULL for all pages, not only the first page. But it doesn't for SG pages. So, SG pages unexpectedly will be reused. It eventually causes kernel panic. Oops: general protection fault, probably for non-canonical address 0x504f4e4dbebc64ff: 0000 [#1] PREEMPT SMP NOPTI CPU: 3 PID: 0 Comm: swapper/3 Not tainted 6.10.0-rc3+ #25 RIP: 0010:xdp_return_frame+0x42/0x90 Code: 01 75 12 5b 4c 89 e6 5d 31 c9 41 5c 31 d2 41 5d e9 73 fd ff ff 44 8b 6b 20 0f b7 43 0a 49 81 ed 68 01 00 00 49 29 c5 49 01 fd <41> 80 7d0 RSP: 0018:ffff99d00122ce08 EFLAGS: 00010202 RAX: 0000000000005453 RBX: ffff8d325f904000 RCX: 0000000000000001 RDX: 00000000670e1000 RSI: 000000011f90d000 RDI: 504f4e4d4c4b4a49 RBP: ffff99d003907740 R08: 0000000000000000 R09: 0000000000000000 R10: 000000011f90d000 R11: 0000000000000000 R12: ffff8d325f904010 R13: 504f4e4dbebc64fd R14: ffff8d3242b070c8 R15: ffff99d0039077c0 FS: 0000000000000000(0000) GS:ffff8d399f780000(0000) knlGS:0000000000000000 CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033 CR2: 00007f41f6c85e38 CR3: 000000037ac30000 CR4: 00000000007506f0 PKRU: 55555554 Call Trace: <IRQ> ? die_addr+0x33/0x90 ? exc_general_protection+0x251/0x2f0 ? asm_exc_general_protection+0x22/0x30 ? xdp_return_frame+0x42/0x90 ionic_tx_clean+0x211/0x280 [ionic 15881354510e6a9c655c59c54812b319ed2cd015] ionic_tx_cq_service+0xd3/0x210 [ionic 15881354510e6a9c655c59c54812b319ed2cd015] ionic_txrx_napi+0x41/0x1b0 [ionic 15881354510e6a9c655c59c54812b319ed2cd015] __napi_poll.constprop.0+0x29/0x1b0 net_rx_action+0x2c4/0x350 handle_softirqs+0xf4/0x320 irq_exit_rcu+0x78/0xa0 common_interrupt+0x77/0x90 Fixes: 5377805dc1c0 ("ionic: implement xdp frags support") Signed-off-by: Taehee Yoo <ap420073@gmail.com> --- v2: - Use ionic_xdp_rx_put_bufs() instead of open code. .../net/ethernet/pensando/ionic/ionic_txrx.c | 27 ++++++++++++------- 1 file changed, 18 insertions(+), 9 deletions(-)