| Message ID | 20210422151620.58204-1-xuanzhuo@linux.alibaba.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | f80bd740cb7c954791279590b2e810ba6c214e52 |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net-next] virtio-net: fix use-after-free in skb_gro_receive | expand |
| Context | Check | Description |
|---|---|---|
| netdev/cover_letter | success | Link |
| netdev/fixes_present | success | Link |
| netdev/patch_count | success | Link |
| netdev/tree_selection | success | Clearly marked for net-next |
| netdev/subject_prefix | success | Link |
| netdev/cc_maintainers | success | CCed 7 of 7 maintainers |
| netdev/source_inline | success | Was 0 now: 0 |
| netdev/verify_signedoff | success | Link |
| netdev/module_param | success | Was 0 now: 0 |
| netdev/build_32bit | success | Errors and warnings before: 0 this patch: 0 |
| netdev/kdoc | success | Errors and warnings before: 0 this patch: 0 |
| netdev/verify_fixes | success | Link |
| netdev/checkpatch | success | total: 0 errors, 0 warnings, 0 checks, 38 lines checked |
| netdev/build_allmodconfig_warn | success | Errors and warnings before: 0 this patch: 0 |
| netdev/header_inline | success | Link |
在 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 > 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; > }
Hello: This patch was applied to netdev/net-next.git (refs/heads/master): On Thu, 22 Apr 2021 23:16:20 +0800 you 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> > > [...] Here is the summary with links: - [net-next] virtio-net: fix use-after-free in skb_gro_receive https://git.kernel.org/netdev/net-next/c/f80bd740cb7c You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
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@linux.alibaba.com> > Reported-by: Ido Schimmel <idosch@nvidia.com> > Tested-by: Ido Schimmel <idosch@nvidia.com> Acked-by: Michael S. Tsirkin <mst@redhat.com> > --- > 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
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; }
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> --- drivers/net/virtio_net.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-)