Re: [net-next,v2,3/6] net: bcmgenet: add basic XDP support (PASS/DROP)

[Nicolai Buchwitz]

On 17.3.2026 17:19, Simon Horman wrote:
> [...]

> > +static struct sk_buff *bcmgenet_xdp_build_skb(struct bcmgenet_rx_ring 
> > *ring,
> > +					      struct xdp_buff *xdp,
> > +					      struct page *rx_page)
> > +{
> > +	unsigned int metasize;
> > +	struct sk_buff *skb;
> > +
> > +	skb = napi_build_skb(xdp->data_hard_start, PAGE_SIZE);
> > +	if (unlikely(!skb))
> > +		return NULL;
> > +
> > +	skb_mark_for_recycle(skb);
> > +
> > +	metasize = xdp->data - xdp->data_meta;
> > +	skb_reserve(skb, xdp->data - xdp->data_hard_start);
> > +	__skb_put(skb, xdp->data_end - xdp->data);
> > +
> > +	if (metasize)
> > +		skb_metadata_set(skb, metasize);
>                           ^^^^
>
> Can this corrupt memory? Looking at bcmgenet_desc_rx(), the xdp_buff is
> initialized with meta_valid=false:
>
> 	xdp_prepare_buff(&xdp, page_address(rx_page),
> 			 GENET_RX_HEADROOM, pkt_len, false);
>
> When meta_valid is false, xdp_prepare_buff() sets xdp->data_meta to
> xdp->data + 1 as a sentinel value to indicate no metadata is present.
>
> In bcmgenet_xdp_build_skb(), the metasize calculation becomes:
>
> 	metasize = xdp->data - xdp->data_meta
> 	         = xdp->data - (xdp->data + 1)
> 	         = (ptrdiff_t)-1
>
> When assigned to 'unsigned int metasize', this wraps to UINT_MAX. The
> 'if (metasize)' check passes since UINT_MAX != 0, and
> skb_metadata_set(skb, metasize) truncates UINT_MAX to u8, setting
> skb_shinfo(skb)->meta_len = 255.
>
> This incorrect meta_len of 255 can cause downstream SKB consumers (TC 
> BPF
> programs, skb_data_move) to read or write 255 bytes before
> skb_mac_header(skb) when accessing the metadata region.
>
> Other XDP drivers that use skb_metadata_set() (bnxt, mvneta, ixgbe,
> i40e, virtio_net) all pass meta_valid=true to xdp_prepare_buff(), which
> sets data_meta=data, giving metasize=0 and correctly skipping
> skb_metadata_set().
>
> Should xdp_prepare_buff() be called with meta_valid=true instead of
> false, since bcmgenet_xdp_build_skb() handles XDP metadata?

Will fix this in v3 by passing meta_valid=true, matching what other XDP
drivers do.

Thanks
Nicolai

> [...]