On Wed, Jun 10, 2026 at 06:21:49PM -0400, Michael S. Tsirkin wrote:
> On Wed, Jun 10, 2026 at 03:16:06PM -0700, Xiang Mei wrote:
> > receive_big() bounds the device-announced length by
> > (big_packets_num_skbfrags + 1) * PAGE_SIZE.  That is still too loose:
> > add_recvbuf_big() sets sg[1] to start at offset
> > sizeof(struct padded_vnet_hdr) into the first page, so the chain
> > actually carries hdr_len + (PAGE_SIZE - sizeof(padded_vnet_hdr)) +
> > big_packets_num_skbfrags * PAGE_SIZE bytes -- 20 bytes less than the
> > check allows for the common hdr_len == 12 case.
> > 
> > A malicious virtio backend can announce a len in that gap.  page_to_skb()
> > then walks one frag past the page chain, storing a NULL page->private
> > into skb_shinfo()->frags[MAX_SKB_FRAGS], which is both an out-of-bounds
> > write past the static frag array and a NULL frag handed up the rx path.
> > Bound len by the size add_recvbuf_big() actually advertised.
> > 
> > Fixes: 0c716703965f ("virtio-net: fix received length check in big packets")
> > Reported-by: Weiming Shi <[email protected]>
> > Signed-off-by: Xiang Mei <[email protected]>
> 
> Let's instead (or additionally?), bound the frags array index.

I went with "additionally" rather than "instead": 1/2 restores the
length invariant in receive_big(), and 2/2 makes the frag loop
self-defending so it no longer depends on the caller validating len.

I didn't find a case triggering the issue of 2/2 so I could not produce
a PoC that actually crashes the kernel through 2/2's path independently
of the 1/2 bug. With 1/2 applied, the over-long len is rejected before
the loop, so the guard is never exercised by a real trigger I could find.

So 2/2 is defense-in-depth rather than a demonstrated separate bug.
Please feel free to decide whether it's worth taking. A v2 was sent.

Thanks,
Xiang
> 
> Seems more robust.
> 
> > ---
> >  drivers/net/virtio_net.c | 8 +++++---
> >  1 file changed, 5 insertions(+), 3 deletions(-)
> > 
> > diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> > index f4adcfee7a80..afe73eda1491 100644
> > --- a/drivers/net/virtio_net.c
> > +++ b/drivers/net/virtio_net.c
> > @@ -1999,15 +1999,17 @@ static struct sk_buff *receive_big(struct 
> > net_device *dev,
> >                                struct virtnet_rq_stats *stats)
> >  {
> >     struct page *page = buf;
> > +   unsigned long max_len;
> >     struct sk_buff *skb;
> >  
> >     /* Make sure that len does not exceed the size allocated in
> >      * add_recvbuf_big.
> >      */
> > -   if (unlikely(len > (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE)) {
> > +   max_len = vi->hdr_len + (PAGE_SIZE - sizeof(struct padded_vnet_hdr)) +
> > +             vi->big_packets_num_skbfrags * PAGE_SIZE;
> > +   if (unlikely(len > max_len)) {
> >             pr_debug("%s: rx error: len %u exceeds allocated size %lu\n",
> > -                    dev->name, len,
> > -                    (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE);
> > +                    dev->name, len, max_len);
> >             goto err;
> >     }
> >  
> > -- 
> > 2.43.0
> 

Reply via email to