Re: [PATCH net-next] virtio-net: fix use-after-free in skb_gro_receive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




在 2021/5/3 下午4:29, Xuan Zhuo 写道:
On Mon, 3 May 2021 04:00:13 -0400, Michael S. Tsirkin<mst@xxxxxxxxxx>  wrote:
On Fri, Apr 23, 2021 at 12:33:09PM +0800, Jason Wang wrote:
在 2021/4/23 下午12:19, Xuan Zhuo 写道:
On Fri, 23 Apr 2021 12:08:34 +0800, Jason Wang<jasowang@xxxxxxxxxx>  wrote:
在 2021/4/22 下午11:16, Xuan Zhuo 写道:
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: Jason Wang<jasowang@xxxxxxxxxx>

The codes became hard to read, I think we can try to do some cleanups on
top to make it easier to read.

Thanks
Yes, this piece of code needs to be sorted out. Especially the big and mergeable
scenarios should be handled separately. Remove the mergeable code from this
function, and mergeable uses a new function alone.
Right, another thing is that we may consider to relax the checking of len <
GOOD_COPY_LEN.
Want to post a patch on top?
Yes, I have completed the refactoring of this part of the code, and the related
testing work is in progress. I will submit the patch after the complete test is
completed.

Thanks.


Cool, one thing in my mind is we'd better pack the correct truesize (e.g PAGE_SIZE in the case of XDP) in the ctx and avoid tricky codes like:

                /* Buffers with headroom use PAGE_SIZE as alloc size,
                 * see add_recvbuf_mergeable() + get_mergeable_buf_len()
                 */

Thanks




_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux