From: Jason Wang <jasowang@redhat.com> To: Li Qiang <liq3ea@gmail.com> Cc: Paolo Bonzini <pbonzini@redhat.com>, Bug 1886362 <1886362@bugs.launchpad.net>, Qemu Developers <qemu-devel@nongnu.org> Subject: Re: [Bug 1886362] [NEW] Heap use-after-free in lduw_he_p through e1000e_write_to_rx_buffers Date: Wed, 15 Jul 2020 16:35:09 +0800 [thread overview] Message-ID: <f19f605c-9468-e7eb-f255-60766df2a50c@redhat.com> (raw) In-Reply-To: <CAKXe6SJb=1YLLGF+TP1fMd95k_WzZd73JeUJv5Sn4EE4m2zP4w@mail.gmail.com> On 2020/7/14 下午6:48, Li Qiang wrote: > Jason Wang <jasowang@redhat.com> 于2020年7月14日周二 下午4:56写道: >> >> On 2020/7/10 下午6:37, Li Qiang wrote: >>> Paolo Bonzini <pbonzini@redhat.com> 于2020年7月10日周五 上午1:36写道: >>>> On 09/07/20 17:51, Li Qiang wrote: >>>>> Maybe we should check whether the address is a RAM address in 'dma_memory_rw'? >>>>> But it is a hot path. I'm not sure it is right. Hope more discussion. >>>> Half of the purpose of dma-helpers.c (as opposed to address_space_* >>>> functions in exec.c) is exactly to support writes to MMIO. This is >>> Hi Paolo, >>> >>> Could you please explain more about this(to support writes to MMIO). >>> I can just see the dma helpers with sg DMA, not related with MMIO. >> >> Please refer doc/devel/memory.rst. >> >> The motivation of memory API is to allow support modeling different >> memory regions. DMA to MMIO is allowed in hardware so Qemu should >> emulate this behaviour. >> > I just read the code again. > So the dma_blk_io is used for some device that will need DMA to > MMIO(may be related with > device spec). But for most of the devices(networking card for > example) there is no need this DMA to MMIO. > So we just ksuse dma_memory_rw. Is this understanding right? > > Then another question. > Though the dma helpers uses a bouncing buffer, it finally write to the > device addressspace in 'address_space_unmap'. > Is there any posibility that we can again write to the MMIO like this issue? I think the point is to make DMA to MMIO work as real hardware. For e1000e and other networking devices we need make sure such DMA doesn't break anything. Thanks > > >>> >>>> especially true of dma_blk_io, which takes care of doing the DMA via a >>>> bounce buffer, possibly in multiple steps and even blocking due to >>>> cpu_register_map_client. >>>> >>>> For dma_memory_rw this is not needed, so it only needs to handle >>>> QEMUSGList, but I think the design should be the same. >>>> >>>> However, this is indeed a nightmare for re-entrancy. The easiest >>>> solution is to delay processing of descriptors to a bottom half whenever >>>> MMIO is doing something complicated. This is also better for latency >>>> because it will free the vCPU thread more quickly and leave the work to >>>> the I/O thread. >>> Do you mean we define a per-e1000e bottom half. And in the MMIO write >>> or packet send >>> trigger this bh? >> >> Probably a TX bh. >> > I will try to write this tx bh to strength my understanding in this part. > Maybe reference the virtio-net implementation I think. > > > > Thanks, > Li Qiang > >>> So even if we again trigger the MMIO write, then >>> second bh will not be executed? >> >> Bh is serialized so no re-entrancy issue. >> >> Thanks >> >> >>> >>> Thanks, >>> Li Qiang >>> >>>> Paolo >>>>
WARNING: multiple messages have this Message-ID (diff)
From: Jason Wang <1886362@bugs.launchpad.net> To: qemu-devel@nongnu.org Subject: Re: [Bug 1886362] [NEW] Heap use-after-free in lduw_he_p through e1000e_write_to_rx_buffers Date: Wed, 15 Jul 2020 08:35:09 -0000 [thread overview] Message-ID: <f19f605c-9468-e7eb-f255-60766df2a50c@redhat.com> (raw) Message-ID: <20200715083509._GDQzbdpnZZvEXHGr0Qapat06Yz0pAL3xErE1VKLsfc@z> (raw) In-Reply-To: 159400349818.1851.7243060688419202620.malonedeb@wampee.canonical.com On 2020/7/14 下午6:48, Li Qiang wrote: > Jason Wang <jasowang@redhat.com> 于2020年7月14日周二 下午4:56写道: >> >> On 2020/7/10 下午6:37, Li Qiang wrote: >>> Paolo Bonzini <pbonzini@redhat.com> 于2020年7月10日周五 上午1:36写道: >>>> On 09/07/20 17:51, Li Qiang wrote: >>>>> Maybe we should check whether the address is a RAM address in 'dma_memory_rw'? >>>>> But it is a hot path. I'm not sure it is right. Hope more discussion. >>>> Half of the purpose of dma-helpers.c (as opposed to address_space_* >>>> functions in exec.c) is exactly to support writes to MMIO. This is >>> Hi Paolo, >>> >>> Could you please explain more about this(to support writes to MMIO). >>> I can just see the dma helpers with sg DMA, not related with MMIO. >> >> Please refer doc/devel/memory.rst. >> >> The motivation of memory API is to allow support modeling different >> memory regions. DMA to MMIO is allowed in hardware so Qemu should >> emulate this behaviour. >> > I just read the code again. > So the dma_blk_io is used for some device that will need DMA to > MMIO(may be related with > device spec). But for most of the devices(networking card for > example) there is no need this DMA to MMIO. > So we just ksuse dma_memory_rw. Is this understanding right? > > Then another question. > Though the dma helpers uses a bouncing buffer, it finally write to the > device addressspace in 'address_space_unmap'. > Is there any posibility that we can again write to the MMIO like this issue? I think the point is to make DMA to MMIO work as real hardware. For e1000e and other networking devices we need make sure such DMA doesn't break anything. Thanks > > >>> >>>> especially true of dma_blk_io, which takes care of doing the DMA via a >>>> bounce buffer, possibly in multiple steps and even blocking due to >>>> cpu_register_map_client. >>>> >>>> For dma_memory_rw this is not needed, so it only needs to handle >>>> QEMUSGList, but I think the design should be the same. >>>> >>>> However, this is indeed a nightmare for re-entrancy. The easiest >>>> solution is to delay processing of descriptors to a bottom half whenever >>>> MMIO is doing something complicated. This is also better for latency >>>> because it will free the vCPU thread more quickly and leave the work to >>>> the I/O thread. >>> Do you mean we define a per-e1000e bottom half. And in the MMIO write >>> or packet send >>> trigger this bh? >> >> Probably a TX bh. >> > I will try to write this tx bh to strength my understanding in this part. > Maybe reference the virtio-net implementation I think. > > > > Thanks, > Li Qiang > >>> So even if we again trigger the MMIO write, then >>> second bh will not be executed? >> >> Bh is serialized so no re-entrancy issue. >> >> Thanks >> >> >>> >>> Thanks, >>> Li Qiang >>> >>>> Paolo >>>> -- You received this bug notification because you are a member of qemu- devel-ml, which is subscribed to QEMU. https://bugs.launchpad.net/bugs/1886362 Title: Heap use-after-free in lduw_he_p through e1000e_write_to_rx_buffers Status in QEMU: New Bug description: Hello, This reproducer causes a heap-use-after free. QEMU Built with --enable-sanitizers: cat << EOF | ./i386-softmmu/qemu-system-i386 -M q35,accel=qtest \ -qtest stdio -nographic -monitor none -serial none outl 0xcf8 0x80001010 outl 0xcfc 0xe1020000 outl 0xcf8 0x80001014 outl 0xcf8 0x80001004 outw 0xcfc 0x7 outl 0xcf8 0x800010a2 write 0xe102003b 0x1 0xff write 0xe1020103 0x1e 0xffffff055c5e5c30be4511d084ffffffffffffffffffffffffffffffffff write 0xe1020420 0x4 0xffffffff write 0xe1020424 0x4 0xffffffff write 0xe102042b 0x1 0xff write 0xe1020430 0x4 0x055c5e5c write 0x5c041 0x1 0x04 write 0x5c042 0x1 0x02 write 0x5c043 0x1 0xe1 write 0x5c048 0x1 0x8a write 0x5c04a 0x1 0x31 write 0x5c04b 0x1 0xff write 0xe1020403 0x1 0xff EOF The Output: ==22689==ERROR: AddressSanitizer: heap-use-after-free on address 0x62500026800e at pc 0x55b93bb18bfa bp 0x7fffdbe844f0 sp 0x7fffdbe83cb8 READ of size 2 at 0x62500026800e thread T0 #0 in __asan_memcpy (/build/i386-softmmu/qemu-system-i386+) #1 in lduw_he_p /include/qemu/bswap.h:332:5 #2 in ldn_he_p /include/qemu/bswap.h:550:1 #3 in flatview_write_continue /exec.c:3145:19 #4 in flatview_write /exec.c:3186:14 #5 in address_space_write /exec.c:3280:18 #6 in address_space_rw /exec.c:3290:16 #7 in dma_memory_rw_relaxed /include/sysemu/dma.h:87:18 #8 in dma_memory_rw /include/sysemu/dma.h:113:12 #9 in pci_dma_rw /include/hw/pci/pci.h:789:5 #10 in pci_dma_write /include/hw/pci/pci.h:802:12 #11 in e1000e_write_to_rx_buffers /hw/net/e1000e_core.c:1412:9 #12 in e1000e_write_packet_to_guest /hw/net/e1000e_core.c:1582:21 #13 in e1000e_receive_iov /hw/net/e1000e_core.c:1709:9 #14 in e1000e_nc_receive_iov /hw/net/e1000e.c:213:12 #15 in net_tx_pkt_sendv /hw/net/net_tx_pkt.c:544:9 #16 in net_tx_pkt_send /hw/net/net_tx_pkt.c:620:9 #17 in net_tx_pkt_send_loopback /hw/net/net_tx_pkt.c:633:11 #18 in e1000e_tx_pkt_send /hw/net/e1000e_core.c:664:16 #19 in e1000e_process_tx_desc /hw/net/e1000e_core.c:743:17 #20 in e1000e_start_xmit /hw/net/e1000e_core.c:934:9 #21 in e1000e_set_tctl /hw/net/e1000e_core.c:2431:9 #22 in e1000e_core_write /hw/net/e1000e_core.c:3265:9 #23 in e1000e_mmio_write /hw/net/e1000e.c:109:5 #24 in memory_region_write_accessor /memory.c:483:5 #25 in access_with_adjusted_size /memory.c:544:18 #26 in memory_region_dispatch_write /memory.c:1476:16 #27 in flatview_write_continue /exec.c:3146:23 #28 in flatview_write /exec.c:3186:14 #29 in address_space_write /exec.c:3280:18 #30 in qtest_process_command /qtest.c:567:9 #31 in qtest_process_inbuf /qtest.c:710:9 #32 in qtest_read /qtest.c:722:5 #33 in qemu_chr_be_write_impl /chardev/char.c:188:9 #34 in qemu_chr_be_write /chardev/char.c:200:9 #35 in fd_chr_read /chardev/char-fd.c:68:9 #36 in qio_channel_fd_source_dispatch /io/channel-watch.c:84:12 #37 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+) #38 in glib_pollfds_poll /util/main-loop.c:219:9 #39 in os_host_main_loop_wait /util/main-loop.c:242:5 #40 in main_loop_wait /util/main-loop.c:518:11 #41 in qemu_main_loop /softmmu/vl.c:1664:9 #42 in main /softmmu/main.c:52:5 #43 in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+) #44 in _start (/build/i386-softmmu/qemu-system-i386+) 0x62500026800e is located 14 bytes inside of 138-byte region [0x625000268000,0x62500026808a) freed by thread T0 here: #0 in free (/build/i386-softmmu/qemu-system-i386+) #1 in qemu_vfree /util/oslib-posix.c:238:5 #2 in address_space_unmap /exec.c:3616:5 #3 in dma_memory_unmap /include/sysemu/dma.h:148:5 #4 in pci_dma_unmap /include/hw/pci/pci.h:839:5 #5 in net_tx_pkt_reset /hw/net/net_tx_pkt.c:453:9 #6 in e1000e_process_tx_desc /hw/net/e1000e_core.c:749:9 #7 in e1000e_start_xmit /hw/net/e1000e_core.c:934:9 #8 in e1000e_set_tctl /hw/net/e1000e_core.c:2431:9 #9 in e1000e_core_write /hw/net/e1000e_core.c:3265:9 #10 in e1000e_mmio_write /hw/net/e1000e.c:109:5 #11 in memory_region_write_accessor /memory.c:483:5 #12 in access_with_adjusted_size /memory.c:544:18 #13 in memory_region_dispatch_write /memory.c:1476:16 #14 in flatview_write_continue /exec.c:3146:23 #15 in flatview_write /exec.c:3186:14 #16 in address_space_write /exec.c:3280:18 #17 in address_space_rw /exec.c:3290:16 #18 in dma_memory_rw_relaxed /include/sysemu/dma.h:87:18 #19 in dma_memory_rw /include/sysemu/dma.h:113:12 #20 in pci_dma_rw /include/hw/pci/pci.h:789:5 #21 in pci_dma_write /include/hw/pci/pci.h:802:12 #22 in e1000e_write_to_rx_buffers /hw/net/e1000e_core.c:1412:9 #23 in e1000e_write_packet_to_guest /hw/net/e1000e_core.c:1582:21 #24 in e1000e_receive_iov /hw/net/e1000e_core.c:1709:9 #25 in e1000e_nc_receive_iov /hw/net/e1000e.c:213:12 #26 in net_tx_pkt_sendv /hw/net/net_tx_pkt.c:544:9 #27 in net_tx_pkt_send /hw/net/net_tx_pkt.c:620:9 #28 in net_tx_pkt_send_loopback /hw/net/net_tx_pkt.c:633:11 #29 in e1000e_tx_pkt_send /hw/net/e1000e_core.c:664:16 previously allocated by thread T0 here: #0 in posix_memalign (/build/i386-softmmu/qemu-system-i386+) #1 in qemu_try_memalign /util/oslib-posix.c:198:11 #2 in qemu_memalign /util/oslib-posix.c:214:27 #3 in address_space_map /exec.c:3558:25 #4 in dma_memory_map /include/sysemu/dma.h:138:9 #5 in pci_dma_map /include/hw/pci/pci.h:832:11 #6 in net_tx_pkt_add_raw_fragment /hw/net/net_tx_pkt.c:391:24 #7 in e1000e_process_tx_desc /hw/net/e1000e_core.c:731:14 #8 in e1000e_start_xmit /hw/net/e1000e_core.c:934:9 #9 in e1000e_set_tctl /hw/net/e1000e_core.c:2431:9 #10 in e1000e_core_write /hw/net/e1000e_core.c:3265:9 #11 in e1000e_mmio_write /hw/net/e1000e.c:109:5 #12 in memory_region_write_accessor /memory.c:483:5 #13 in access_with_adjusted_size /memory.c:544:18 #14 in memory_region_dispatch_write /memory.c:1476:16 #15 in flatview_write_continue /exec.c:3146:23 #16 in flatview_write /exec.c:3186:14 #17 in address_space_write /exec.c:3280:18 #18 in qtest_process_command /qtest.c:567:9 #19 in qtest_process_inbuf /qtest.c:710:9 #20 in qtest_read /qtest.c:722:5 #21 in qemu_chr_be_write_impl /chardev/char.c:188:9 #22 in qemu_chr_be_write /chardev/char.c:200:9 #23 in fd_chr_read /chardev/char-fd.c:68:9 #24 in qio_channel_fd_source_dispatch /io/channel-watch.c:84:12 #25 in g_main_context_dispatch (/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0+) -Alex To manage notifications about this bug go to: https://bugs.launchpad.net/qemu/+bug/1886362/+subscriptions
next prev parent reply other threads:[~2020-07-15 8:36 UTC|newest] Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-06 2:44 [Bug 1886362] [NEW] Heap use-after-free in lduw_he_p through e1000e_write_to_rx_buffers Alexander Bulekov 2020-07-07 4:52 ` [Bug 1886362] " Philippe Mathieu-Daudé 2020-07-09 15:51 ` [Bug 1886362] [NEW] " Li Qiang 2020-07-09 17:36 ` Paolo Bonzini 2020-07-10 10:37 ` Li Qiang 2020-07-14 8:56 ` Jason Wang 2020-07-14 8:56 ` Jason Wang 2020-07-14 10:48 ` Li Qiang 2020-07-15 8:35 ` Jason Wang [this message] 2020-07-15 8:35 ` Jason Wang 2020-07-21 12:31 ` Peter Maydell 2020-07-21 12:31 ` Peter Maydell 2020-07-21 13:21 ` Jason Wang 2020-07-21 13:21 ` Jason Wang 2020-07-21 13:44 ` Peter Maydell 2020-07-21 13:44 ` Peter Maydell 2020-07-21 14:37 ` Alexander Bulekov 2020-07-21 14:37 ` Alexander Bulekov 2020-07-22 3:25 ` Jason Wang 2020-07-22 3:25 ` Jason Wang 2020-07-21 13:46 ` Li Qiang 2020-07-22 3:27 ` Jason Wang 2020-07-22 3:27 ` Jason Wang 2020-07-21 12:14 ` [Bug 1886362] " P J P 2021-05-26 14:34 ` Thomas Huth 2021-06-14 23:40 ` Alexander Bulekov 2021-06-15 18:19 ` Thomas Huth
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=f19f605c-9468-e7eb-f255-60766df2a50c@redhat.com \ --to=jasowang@redhat.com \ --cc=1886362@bugs.launchpad.net \ --cc=liq3ea@gmail.com \ --cc=pbonzini@redhat.com \ --cc=qemu-devel@nongnu.org \ /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 a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).