| 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
| ||
|
Message-ID: <e21a3087-0c2f-4087-bb77-ed9cff2d5ebb@amd.com>
Date: Thu, 20 Jun 2024 14:47:53 -0700
From: "Nelson, Shannon" <shannon.nelson@....com>
To: Taehee Yoo <ap420073@...il.com>, davem@...emloft.net, kuba@...nel.org,
pabeni@...hat.com, edumazet@...gle.com, brett.creeley@....com,
drivers@...sando.io, netdev@...r.kernel.org
Cc: jacob.e.keller@...el.com
Subject: Re: [PATCH net v2] ionic: fix kernel panic due to multi-buffer
handling
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@...il.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@....com>
> --
> 2.34.1
>
Powered by blists - more mailing lists