Re: [PATCH bpf v2] veth: convert frag_list skbs before running XDP

[Lorenzo Bianconi <lorenzo@kernel.org>]

[-- Attachment #1: Type: text/plain, Size: 3615 bytes --]

> From: Matt Fleming <mfleming@cloudflare.com>
> 
> A frag_list skb can reach veth with data_len set but nr_frags zero.
> veth_convert_skb_to_xdp_buff() only converts skbs that are shared,
> locked, have frags[], or do not have enough headroom. It later uses
> skb_is_nonlinear() to decide whether to set XDP_FLAGS_HAS_FRAGS and
> xdp_frags_size.
> 
> That exposes frag_list data to XDP as if it were stored in frags[], but
> frags[] is empty. AF_XDP copy mode can then trust the bogus XDP fragment
> metadata, walk an empty fragment entry, and crash in memcpy() from
> __xsk_rcv().
> 
> Route non-linear skbs through skb_pp_cow_data() before exposing them to
> XDP, and only advertise XDP frags when the resulting skb has frags[].
> skb_copy_bits() already handles frag_list input, and skb_pp_cow_data()
> builds frags[] output with skb_add_rx_frag(), which is the
> representation XDP multi-buffer expects.
> 
> Fixes: 718a18a0c8a6 ("veth: Rework veth_xdp_rcv_skb in order to accept non-linear skb")
> Cc: stable@vger.kernel.org
> Signed-off-by: Matt Fleming <mfleming@cloudflare.com>

Acked-by: Lorenzo Bianconi <lorenzo@kernel.org>

> ---
> Changes in v2:
> - Use skb_is_nonlinear() in veth_convert_skb_to_xdp_buff().
> - Move the skb_pp_cow_data() comment into kerneldoc.
> 
>  drivers/net/veth.c |  4 ++--
>  net/core/skbuff.c  | 16 ++++++++++------
>  2 files changed, 12 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/veth.c b/drivers/net/veth.c
> index 1c5142149175..00e34afd858e 100644
> --- a/drivers/net/veth.c
> +++ b/drivers/net/veth.c
> @@ -756,7 +756,7 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
>  	u32 frame_sz;
>  
>  	if (skb_shared(skb) || skb_head_is_locked(skb) ||
> -	    skb_shinfo(skb)->nr_frags ||
> +	    skb_is_nonlinear(skb) ||
>  	    skb_headroom(skb) < XDP_PACKET_HEADROOM) {
>  		if (skb_pp_cow_data(rq->page_pool, pskb, XDP_PACKET_HEADROOM))
>  			goto drop;
> @@ -771,7 +771,7 @@ static int veth_convert_skb_to_xdp_buff(struct veth_rq *rq,
>  	xdp_prepare_buff(xdp, skb->head, skb_headroom(skb),
>  			 skb_headlen(skb), true);
>  
> -	if (skb_is_nonlinear(skb)) {
> +	if (skb_shinfo(skb)->nr_frags) {
>  		skb_shinfo(skb)->xdp_frags_size = skb->data_len;
>  		xdp_buff_set_frags_flag(xdp);
>  	} else {
> diff --git a/net/core/skbuff.c b/net/core/skbuff.c
> index 18dabb4e9cfa..66f57131633a 100644
> --- a/net/core/skbuff.c
> +++ b/net/core/skbuff.c
> @@ -927,6 +927,16 @@ static void skb_clone_fraglist(struct sk_buff *skb)
>  		skb_get(list);
>  }
>  
> +/**
> + * skb_pp_cow_data() - copy skb data into page-pool backed storage
> + * @pool: page pool to allocate from
> + * @pskb: pointer to skb pointer, replaced with the copied skb on success
> + * @headroom: headroom to reserve in the copied skb
> + *
> + * skb_copy_bits() handles both frags[] and frag_list input. If the copied
> + * skb remains non-linear, it uses frags[], which is the representation used
> + * by XDP multi-buffer.
> + */
>  int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb,
>  		    unsigned int headroom)
>  {
> @@ -936,12 +946,6 @@ int skb_pp_cow_data(struct page_pool *pool, struct sk_buff **pskb,
>  	int err, i, head_off;
>  	void *data;
>  
> -	/* XDP does not support fraglist so we need to linearize
> -	 * the skb.
> -	 */
> -	if (skb_has_frag_list(skb))
> -		return -EOPNOTSUPP;
> -
>  	max_head_size = SKB_WITH_OVERHEAD(PAGE_SIZE - headroom);
>  	if (skb->len > max_head_size + MAX_SKB_FRAGS * PAGE_SIZE)
>  		return -ENOMEM;
> -- 
> 2.43.0
> 

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]