From: Jason Wang <jasowang@redhat.com> To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>, "Michael S. Tsirkin" <mst@redhat.com> Cc: "David S. Miller" <davem@davemloft.net>, Jakub Kicinski <kuba@kernel.org>, virtualization@lists.linux-foundation.org, Ido Schimmel <idosch@nvidia.com>, netdev@vger.kernel.org Subject: Re: [PATCH net-next] virtio-net: fix use-after-free in skb_gro_receive Date: Thu, 6 May 2021 11:23:32 +0800 [thread overview] Message-ID: <770ce8e6-4948-2977-4d63-7d82075e7fc8@redhat.com> (raw) In-Reply-To: <1620030574.9881887-1-xuanzhuo@linux.alibaba.com> 在 2021/5/3 下午4:29, Xuan Zhuo 写道: > On Mon, 3 May 2021 04:00:13 -0400, Michael S. Tsirkin<mst@redhat.com> 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@redhat.com> 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@linux.alibaba.com> >>>>>> Reported-by: Ido Schimmel<idosch@nvidia.com> >>>>>> Tested-by: Ido Schimmel<idosch@nvidia.com> >>>>>> --- >>>>> Acked-by: Jason Wang<jasowang@redhat.com> >>>>> >>>>> 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 >
WARNING: multiple messages have this Message-ID (diff)
From: Jason Wang <jasowang@redhat.com> To: Xuan Zhuo <xuanzhuo@linux.alibaba.com>, "Michael S. Tsirkin" <mst@redhat.com> Cc: Jakub Kicinski <kuba@kernel.org>, Ido Schimmel <idosch@nvidia.com>, netdev@vger.kernel.org, "David S. Miller" <davem@davemloft.net>, virtualization@lists.linux-foundation.org Subject: Re: [PATCH net-next] virtio-net: fix use-after-free in skb_gro_receive Date: Thu, 6 May 2021 11:23:32 +0800 [thread overview] Message-ID: <770ce8e6-4948-2977-4d63-7d82075e7fc8@redhat.com> (raw) In-Reply-To: <1620030574.9881887-1-xuanzhuo@linux.alibaba.com> 在 2021/5/3 下午4:29, Xuan Zhuo 写道: > On Mon, 3 May 2021 04:00:13 -0400, Michael S. Tsirkin<mst@redhat.com> 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@redhat.com> 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@linux.alibaba.com> >>>>>> Reported-by: Ido Schimmel<idosch@nvidia.com> >>>>>> Tested-by: Ido Schimmel<idosch@nvidia.com> >>>>>> --- >>>>> Acked-by: Jason Wang<jasowang@redhat.com> >>>>> >>>>> 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@lists.linux-foundation.org https://lists.linuxfoundation.org/mailman/listinfo/virtualization
next parent reply other threads:[~2021-05-06 3:23 UTC|newest] Thread overview: 12+ messages / expand[flat|nested] mbox.gz Atom feed top [not found] <1620030574.9881887-1-xuanzhuo@linux.alibaba.com> 2021-05-06 3:23 ` Jason Wang [this message] 2021-05-06 3:23 ` [PATCH net-next] virtio-net: fix use-after-free in skb_gro_receive Jason Wang [not found] <1619151585.3098595-1-xuanzhuo@linux.alibaba.com> 2021-04-23 4:33 ` Jason Wang 2021-04-23 4:33 ` Jason Wang 2021-05-03 8:00 ` Michael S. Tsirkin 2021-05-03 8:00 ` Michael S. Tsirkin 2021-04-22 15:16 Xuan Zhuo 2021-04-23 4:08 ` Jason Wang 2021-04-23 4:08 ` Jason Wang 2021-04-23 20:20 ` patchwork-bot+netdevbpf 2021-05-03 7:59 ` Michael S. Tsirkin 2021-05-03 7:59 ` Michael S. Tsirkin
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=770ce8e6-4948-2977-4d63-7d82075e7fc8@redhat.com \ --to=jasowang@redhat.com \ --cc=davem@davemloft.net \ --cc=idosch@nvidia.com \ --cc=kuba@kernel.org \ --cc=mst@redhat.com \ --cc=netdev@vger.kernel.org \ --cc=virtualization@lists.linux-foundation.org \ --cc=xuanzhuo@linux.alibaba.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.