On Thu, Apr 22, 2021 at 11:16:20PM +0800, Xuan Zhuo wrote:
> When "headroom" > 0, the actual allocated memory space is the entire
> page, so the address of the page should be used when passing it to
> build_skb().
>
> BUG: KASAN: use-after-free in skb_gro_receive (net/core/skbuff.c:4260)
> Write of size 16 at addr ffff88811619fffc by task kworker/u9:0/534
> CPU: 2 PID: 534 Comm: kworker/u9:0 Not tainted 5.12.0-rc7-custom-16372-gb150be05b806 #3382
> Hardware name: QEMU MSN2700, BIOS rel-1.13.0-0-gf21b5a4aeb02-prebuilt.qemu.org 04/01/2014
> Workqueue: xprtiod xs_stream_data_receive_workfn [sunrpc]
> Call Trace:
> <IRQ>
> dump_stack (lib/dump_stack.c:122)
> print_address_description.constprop.0 (mm/kasan/report.c:233)
> kasan_report.cold (mm/kasan/report.c:400 mm/kasan/report.c:416)
> skb_gro_receive (net/core/skbuff.c:4260)
> tcp_gro_receive (net/ipv4/tcp_offload.c:266 (discriminator 1))
> tcp4_gro_receive (net/ipv4/tcp_offload.c:316)
> inet_gro_receive (net/ipv4/af_inet.c:1545 (discriminator 2))
> dev_gro_receive (net/core/dev.c:6075)
> napi_gro_receive (net/core/dev.c:6168 net/core/dev.c:6198)
> receive_buf (drivers/net/virtio_net.c:1151) virtio_net
> virtnet_poll (drivers/net/virtio_net.c:1415 drivers/net/virtio_net.c:1519) virtio_net
> __napi_poll (net/core/dev.c:6964)
> net_rx_action (net/core/dev.c:7033 net/core/dev.c:7118)
> __do_softirq (./arch/x86/include/asm/jump_label.h:25 ./include/linux/jump_label.h:200 ./include/trace/events/irq.h:142 kernel/softirq.c:346)
> irq_exit_rcu (kernel/softirq.c:221 kernel/softirq.c:422 kernel/softirq.c:434)
> common_interrupt (arch/x86/kernel/irq.c:240 (discriminator 14))
> </IRQ>
>
> Fixes: fb32856b16ad ("virtio-net: page_to_skb() use build_skb when there's sufficient tailroom")
> Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> Reported-by: Ido Schimmel <idosch@xxxxxxxxxx>
> Tested-by: Ido Schimmel <idosch@xxxxxxxxxx>
Acked-by: Michael S. Tsirkin <mst@xxxxxxxxxx>
> ---
> drivers/net/virtio_net.c | 12 +++++++++---
> 1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/net/virtio_net.c b/drivers/net/virtio_net.c
> index 74d2d49264f3..7fda2ae4c40f 100644
> --- a/drivers/net/virtio_net.c
> +++ b/drivers/net/virtio_net.c
> @@ -387,7 +387,7 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> unsigned int copy, hdr_len, hdr_padded_len;
> struct page *page_to_free = NULL;
> int tailroom, shinfo_size;
> - char *p, *hdr_p;
> + char *p, *hdr_p, *buf;
>
> p = page_address(page) + offset;
> hdr_p = p;
> @@ -403,11 +403,15 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
> * space are aligned.
> */
> if (headroom) {
> - /* The actual allocated space size is PAGE_SIZE. */
> + /* Buffers with headroom use PAGE_SIZE as alloc size,
> + * see add_recvbuf_mergeable() + get_mergeable_buf_len()
> + */
> truesize = PAGE_SIZE;
> tailroom = truesize - len - offset;
> + buf = page_address(page);
> } else {
> tailroom = truesize - len;
> + buf = p;
> }
>
> len -= hdr_len;
> @@ -416,11 +420,13 @@ static struct sk_buff *page_to_skb(struct virtnet_info *vi,
>
> shinfo_size = SKB_DATA_ALIGN(sizeof(struct skb_shared_info));
>
> + /* copy small packet so we can reuse these pages */
> if (!NET_IP_ALIGN && len > GOOD_COPY_LEN && tailroom >= shinfo_size) {
> - skb = build_skb(p, truesize);
> + skb = build_skb(buf, truesize);
> if (unlikely(!skb))
> return NULL;
>
> + skb_reserve(skb, p - buf);
> skb_put(skb, len);
> goto ok;
> }
> --
> 2.31.0
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
