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

[Nicolai Buchwitz <nb@tipi-net.de>]

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

> [...]