Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
New issues: - [High] Bypassing pre-allocated buffers when XDP is attached defeats anti-OOM protections, risking permanent interface disablement. Pre-existing issues: - [Critical] Dangling pointer in apc->bpf_prog leading to Use-After-Free and Double Free if mana_pre_alloc_rxbufs fails. -- commit 47baeb3edbb2306dafb469762d4ed1350e2c78f3 Author: Gunter Woytowitz <[email protected]> net: mana: never hand a non-page_pool buffer to XDP This commit ensures that all buffers handled by XDP are allocated from the page_pool to prevent warnings when __xdp_return() frees them. It bypasses the pre-allocated buffers when an XDP program is attached, falling back to dynamic allocation. Unused pre-allocated buffers are safely freed by existing cleanup paths. > diff --git a/drivers/net/ethernet/microsoft/mana/mana_en.c > b/drivers/net/ethernet/microsoft/mana/mana_en.c > index 7a1ac853e3abc..a2cb667530a06 100644 > --- a/drivers/net/ethernet/microsoft/mana/mana_en.c > +++ b/drivers/net/ethernet/microsoft/mana/mana_en.c > @@ -2925,7 +2925,14 @@ static int mana_fill_rx_oob(struct mana_recv_buf_oob > *rx_oob, u32 mem_key, > dma_addr_t da; > void *va; > > - if (mpc->rxbufs_pre) > + /* The pre-allocated buffers come from dev_alloc_pages(), not from the > + * rxq's page_pool. With a program attached any buffer can reach > + * __xdp_return(), which returns it to the pool the rxq registered, so > + * fill from the pool instead. The unused pre-allocated buffers are > + * released by the mana_pre_dealloc_rxbufs() every caller already runs > + * after mana_attach(). > + */ > + if (mpc->rxbufs_pre && !mana_xdp_get(mpc)) > va = mana_get_rxbuf_pre(rxq, &da); > else > va = mana_get_rxfrag(rxq, dev, &da, &from_pool, &pp_page, [Severity: High] Does bypassing the pre-allocated buffers when XDP is attached defeat the anti-OOM protections? When attaching an XDP program under memory pressure, mana_pre_alloc_rxbufs() succeeds, but the dynamic page_pool allocations in mana_fill_rx_oob() can now fail. If mana_attach() fails halfway through due to these allocation failures: drivers/net/ethernet/microsoft/mana/mana_bpf.c:mana_xdp_set() { ... err = mana_attach(ndev); if (err) { netdev_err(ndev, "mana_attach failed at xdp set: %d\n", err); NL_SET_ERR_MSG_MOD(extack, "XDP: Re-config failed at attach"); goto err_dealloc_rxbuffs; } ... } Could this leave the network interface permanently detached until manually reset, effectively doubling the peak memory requirement during the XDP attach process? [Severity: Critical] This is a pre-existing issue, but I noticed a potential use-after-free in mana_xdp_set() during error handling. drivers/net/ethernet/microsoft/mana/mana_bpf.c:mana_xdp_set() { ... apc->bpf_prog = prog; if (apc->port_is_up) { /* Pre-allocate buffers to prevent failure in mana_attach */ err = mana_pre_alloc_rxbufs(apc, ndev->mtu, apc->num_queues); if (err) { NL_SET_ERR_MSG_MOD(extack, "XDP: Insufficient memory for tx/rx re-config"); return err; } ... } If mana_pre_alloc_rxbufs() fails, we return early without restoring apc->bpf_prog to old_prog. Can this leave a dangling pointer in apc->bpf_prog? The caller dev_xdp_attach() in net/core/dev.c drops the reference to the program on error, freeing it. A subsequent call to mana_xdp_set() will read this pointer as old_prog and call bpf_prog_put(old_prog). Will this result in a double-free of the BPF program? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=1
