[PATCH net] virtio-net: fix frag overflow in big packets receive

[PATCH net] virtio-net: fix frag overflow in big packets receive

[Willem de Bruijn]

From: Willem de Bruijn <willemb@google.com>

In big packets mode with guest GSO, add_recvbuf_big() posts a chain of
MAX_SKB_FRAGS + 1 pages. page_to_skb() copies only ETH_HLEN of the
first page into the linear area and adds the rest of that page as a
frag, then one frag per remaining page. A device that fills the entire
buffer therefore causes MAX_SKB_FRAGS + 1 frags, writing one
skb_frag_t past the end of skb_shinfo(skb)->frags[].

Post one page less. The buffer still fits a maximal GSO packet. A well
behaved device does not trigger this, as GSO packets are at most 64KB.

Also check the frag count in page_to_skb() before adding any frag as
defense in depth.

Fixes: 9ab86bbcf8be ("virtio_net: Defer skb allocation in receive path Date: Wed, 13 Jan 2010 12:53:38 -0800")
Signed-off-by: Willem de Bruijn <willemb@google.com>
---

The "Date: ..." in the Fixes tag is literally part of the subject
line of commit 9ab86bbcf8be in git history.

Reported by an offline AI review of a related virtio-net patch.
---
 drivers/net/virtio_net.c | 12 +++++++++++-
 1 file changed, 11 insertions(+), 1 deletion(-)

diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
index daab43ac92ce..cb7ed53fd55e 100644
--- a/drivers/net/virtio_net.c
+++ b/drivers/net/virtio_net.c
@@ -906,6 +906,15 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
 	}
 
 	BUG_ON(offset >= PAGE_SIZE);
+
+	/* The first frag starts at offset, each next frag is a full page. */
+	if (unlikely(DIV_ROUND_UP(offset + len, PAGE_SIZE) > MAX_SKB_FRAGS)) {
+		pr_debug("%s: rx error: len %u needs more than %d frags\n",
+			 vi->dev->name, len, MAX_SKB_FRAGS);
+		kfree_skb(skb);
+		return NULL;
+	}
+
 	while (len) {
 		unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len);
 		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset,
@@ -6717,7 +6726,8 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
 	 */
 	if (mtu > ETH_DATA_LEN || guest_gso) {
 		vi->big_packets = true;
-		vi->big_packets_num_skbfrags = guest_gso ? MAX_SKB_FRAGS : DIV_ROUND_UP(mtu, PAGE_SIZE);
+		vi->big_packets_num_skbfrags = guest_gso ? MAX_SKB_FRAGS - 1 :
+							   DIV_ROUND_UP(mtu, PAGE_SIZE);
 	}
 }
 
-- 
2.56.0.212.g129ede20d9-goog

Re: [PATCH net] virtio-net: fix frag overflow in big packets receive

[Michael S. Tsirkin]

On Mon, Oct 19, 2026 at 11:15:50PM -0400, Willem de Bruijn wrote:
> From: Willem de Bruijn <willemb@google.com>
> 
> In big packets mode with guest GSO, add_recvbuf_big() posts a chain of
> MAX_SKB_FRAGS + 1 pages. page_to_skb() copies only ETH_HLEN of the
> first page into the linear area and adds the rest of that page as a
> frag, then one frag per remaining page. A device that fills the entire
> buffer therefore causes MAX_SKB_FRAGS + 1 frags, writing one
> skb_frag_t past the end of skb_shinfo(skb)->frags[].
> 
> Post one page less. The buffer still fits a maximal GSO packet. A well
> behaved device does not trigger this, as GSO packets are at most 64KB.
> 
> Also check the frag count in page_to_skb() before adding any frag as
> defense in depth.
> 
> Fixes: 9ab86bbcf8be ("virtio_net: Defer skb allocation in receive path Date: Wed, 13 Jan 2010 12:53:38 -0800")

Wow an old one.


> Signed-off-by: Willem de Bruijn <willemb@google.com>


Acked-by: Michael S. Tsirkin <mst@redhat.com>

> ---
> 
> The "Date: ..." in the Fixes tag is literally part of the subject
> line of commit 9ab86bbcf8be in git history.

Caused me a double take, too.

> 
> Reported by an offline AI review of a related virtio-net patch.
> ---
>  drivers/net/virtio_net.c | 12 +++++++++++-
>  1 file changed, 11 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index daab43ac92ce..cb7ed53fd55e 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -906,6 +906,15 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>  	}
>  
>  	BUG_ON(offset >= PAGE_SIZE);
> +
> +	/* The first frag starts at offset, each next frag is a full page. */
> +	if (unlikely(DIV_ROUND_UP(offset + len, PAGE_SIZE) > MAX_SKB_FRAGS)) {
> +		pr_debug("%s: rx error: len %u needs more than %d frags\n",
> +			 vi->dev->name, len, MAX_SKB_FRAGS);
> +		kfree_skb(skb);
> +		return NULL;
> +	}
> +
>  	while (len) {
>  		unsigned int frag_size = min((unsigned)PAGE_SIZE - offset, len);
>  		skb_add_rx_frag(skb, skb_shinfo(skb)->nr_frags, page, offset,
> @@ -6717,7 +6726,8 @@ static void virtnet_set_big_packets(struct virtnet_info *vi, const int mtu)
>  	 */
>  	if (mtu > ETH_DATA_LEN || guest_gso) {
>  		vi->big_packets = true;
> -		vi->big_packets_num_skbfrags = guest_gso ? MAX_SKB_FRAGS : DIV_ROUND_UP(mtu, PAGE_SIZE);
> +		vi->big_packets_num_skbfrags = guest_gso ? MAX_SKB_FRAGS - 1 :
> +							   DIV_ROUND_UP(mtu, PAGE_SIZE);

The two most common issues in computer science are off by one bugs.


>  	}
>  }
>  
> -- 
> 2.56.0.212.g129ede20d9-goog