Re: [PATCH 1/2] bpf: preserve rx_queue_index across XDP redirects

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:

New issues:
- [High] Uninitialized `rx_queue_index` leak in the BPF `test_run` framework exposes kernel memory to BPF programs.

Pre-existing issues:
- [Critical] Incomplete metadata copy when manually duplicating `xdp_frame` in the DPAA driver causes uninitialized flags usage.
- [Critical] Converting a zero-copy XDP buffer with metadata into an `xdp_frame` causes an out-of-bounds write when an SKB is later built from it.
--

commit bc57980ef3c6d7e89349b8a948f430b656d7ecab
Author: Siddharth C <siddharthcibi@icloud.com>

bpf: preserve rx_queue_index across XDP redirects

Store rx_queue_index in struct xdp_frame during xdp_buff to xdp_frame
conversion and restore it when rebuilding xdp_rxq_info for cpumap and
devmap execution paths. This preserves ingress RX queue information for
XDP programs executed after redirect.

> diff --git a/include/net/xdp.h b/include/net/xdp.h
> index aa742f413c358..90318b2b76dcb 100644
> --- a/include/net/xdp.h
> +++ b/include/net/xdp.h
> @@ -301,6 +301,7 @@ struct xdp_frame {
>  	 */
>  	enum xdp_mem_type mem_type:32;
>  	struct net_device *dev_rx; /* used by cpumap */
> +	u32 rx_queue_index;

[Severity: High]
Does adding rx_queue_index here introduce a kernel memory leak in the BPF
test_run framework?

Looking at net/bpf/test_run.c, both xdp_test_run_init_page() and reset_ctx()
manually construct an xdp_frame using xdp_update_frame_from_buff().

Because xdp_update_frame_from_buff() does not copy rx_queue_index, and the
test_run code does not explicitly initialize it, frm->rx_queue_index appears
to retain uninitialized kernel memory from the page pool allocation.

If a test BPF program returns XDP_REDIRECT to a cpumap or devmap, will this
uninitialized rx_queue_index be exposed to the secondary BPF program via the
ctx->rx_queue_index context field, leaking 4 bytes of kernel memory per packet?

[Severity: Critical]
This is a pre-existing issue, but could manually duplicating the xdp_frame
cause uninitialized flags usage in the DPAA driver?

In drivers/net/ethernet/freescale/dpaa/dpaa_eth.c, dpaa_a050385_wa_xdpf()
manually duplicates an xdp_frame into a newly allocated buffer using
dev_alloc_pages(0).

It appears to leave several fields uninitialized, particularly flags. Since
the allocated page is not zeroed, new_xdpf->flags contains uninitialized
memory.

When xdp_return_frame() is later called on this frame, it checks for
XDP_FLAGS_HAS_FRAGS. If the uninitialized memory happens to have this flag
set, won't xdp_return_frame() read a bogus skb_shared_info from the end of
the page and attempt to free random memory addresses as fragments?

>  	u32 frame_sz;
>  	u32 flags; /* supported values defined in xdp_buff_flags */
>  };

[ ... ]

> diff --git a/net/core/xdp.c b/net/core/xdp.c
> index 9890a30584ba7..9691d8dfadf3c 100644
> --- a/net/core/xdp.c
> +++ b/net/core/xdp.c
> @@ -606,6 +606,7 @@ struct xdp_frame *xdp_convert_zc_to_xdp_frame(struct xdp_buff *xdp)
>  	xdpf->metasize = metasize;
>  	xdpf->frame_sz = PAGE_SIZE;
>  	xdpf->mem_type = MEM_TYPE_PAGE_ORDER0;
> +	xdpf->rx_queue_index = xdp->rxq->queue_index;

[Severity: Critical]
This isn't a bug introduced by this patch, but will setting headroom to 0
instead of metasize here cause an out-of-bounds write later?

Looking at xdp_convert_zc_to_xdp_frame(), it sets xdpf->headroom = 0 and
calculates the data offset using metasize.

Later, when __xdp_build_skb_from_frame() processes this frame, it calculates:

hard_start = xdpf->data - (sizeof(*xdpf) + xdpf->headroom)

which evaluates to page_virt + metasize. It then calls
build_skb_around(skb, hard_start, PAGE_SIZE).

This places the skb_shared_info structure at hard_start + PAGE_SIZE (minus
alignment). Because hard_start is offset by metasize, wouldn't the end of the
skb_shared_info overlap into the adjacent physical page by metasize bytes,
corrupting memory when the kernel zero-initializes this out-of-bounds area?

>  
>  	xsk_buff_free(xdp);
>  	return xdpf;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260619195759.41254-1-siddharthcibi@icloud.com?part=1