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 Signed-off-by: Xiang Mei Reviewed-by: Xuan Zhuo --- v4: use easy to understand math to compute the max_len v3: revoke 2/2 and add Xuan Zhuo's Reviewed-by tag v2: add additiona check as 2/2 drivers/net/virtio_net.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c index f4adcfee7a80..8f4562316aaa 100644 --- a/drivers/net/virtio_net.c +++ b/drivers/net/virtio_net.c @@ -1999,15 +1999,16 @@ static struct sk_buff *receive_big(struct net_device *dev, struct virtnet_rq_stats *stats) { struct page *page = buf; + unsigned long max_len = (vi->big_packets_num_skbfrags + 1) * PAGE_SIZE - + sizeof(struct padded_vnet_hdr) + vi->hdr_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)) { + 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