Hello Jason, all +-- On Fri, 17 Jul 2020, Jason Wang wrote --+ | On 2020/7/17 上午9:21, Alexander Bulekov wrote: | > On 200717 0853, Li Qiang wrote: | >> Which issue are you trying to solve, any reference linking? | >> I also send a patch related this part and also a UAF. | > | > I reported a UAF privately to QEMU-Security in May. I believe the one Li | > is referring to is this one https://bugs.launchpad.net/qemu/+bug/1886362 | > | > When I saw Prasad's email, I was worried that I reported the same bug | > twice, but I can still reproduce LP#1886362 with Prasad's patch. | > | > On the other hand, I cannot reproduce either issue with Li's patch: | > Message-Id: <20200716161453.61295-1-liq...@163.com> | > | > Based on this, I think there were two distinct issues.
Yes, LP#1886362 looks different that the one fixed here. | Could you describe the issue you saw in details? (E.g the calltrace?) The | commit log does not explain where we can get OOB or UAF. I should've included the backtrace in the commit message. It crossed my mind after I sent the patch. Sorry. === 1040323==ERROR: AddressSanitizer: heap-use-after-free on address 0x6060000344d8 at pc 0x5571b8fb9ce7 bp 0x7ffede5a2290 sp 0x7ffede5a2280 READ of size 8 at 0x6060000344d8 thread T0 #0 0x5571b8fb9ce6 in e1000e_write_packet_to_guest hw/net/e1000e_core.c:1587 #1 0x5571b8fba8fc in e1000e_receive_iov hw/net/e1000e_core.c:1709 #2 0x5571b8f9cb08 in e1000e_nc_receive_iov hw/net/e1000e.c:213 #3 0x5571b8f90285 in net_tx_pkt_sendv hw/net/net_tx_pkt.c:544 #4 0x5571b8f90aaa in net_tx_pkt_send hw/net/net_tx_pkt.c:620 #5 0x5571b8f90b2e in net_tx_pkt_send_loopback hw/net/net_tx_pkt.c:633 #6 0x5571b8fb3c3b in e1000e_tx_pkt_send hw/net/e1000e_core.c:664 #7 0x5571b8fb43dd in e1000e_process_tx_desc hw/net/e1000e_core.c:743 #8 0x5571b8fb5b4f in e1000e_start_xmit hw/net/e1000e_core.c:934 #9 0x5571b8fbe2ad in e1000e_set_tdt hw/net/e1000e_core.c:2451 #10 0x5571b8fc01e0 in e1000e_core_write hw/net/e1000e_core.c:3265 #11 0x5571b8f9c387 in e1000e_mmio_write hw/net/e1000e.c:109 ... 0x6060000344d8 is located 24 bytes inside of 64-byte region [0x6060000344c0,0x606000034500) freed by thread T0 here: #0 0x7f89e3b87307 in __interceptor_free (/lib64/libasan.so.6+0xb0307) #1 0x7f89e37c7a6c in g_free (/lib64/libglib-2.0.so.0+0x58a6c) #2 0x5571b8f95fc7 in net_rx_pkt_pull_data hw/net/net_rx_pkt.c:103 #3 0x5571b8f969b3 in net_rx_pkt_attach_iovec_ex hw/net/net_rx_pkt.c:158 #4 0x5571b8fba6bc in e1000e_receive_iov hw/net/e1000e_core.c:1695 #5 0x5571b8f9cb08 in e1000e_nc_receive_iov hw/net/e1000e.c:213 #6 0x5571b8f90285 in net_tx_pkt_sendv hw/net/net_tx_pkt.c:544 #7 0x5571b8f90aaa in net_tx_pkt_send hw/net/net_tx_pkt.c:620 #8 0x5571b8f90b2e in net_tx_pkt_send_loopback hw/net/net_tx_pkt.c:633 #9 0x5571b8fb3c3b in e1000e_tx_pkt_send hw/net/e1000e_core.c:664 #10 0x5571b8fb43dd in e1000e_process_tx_desc hw/net/e1000e_core.c:743 #11 0x5571b8fb5b4f in e1000e_start_xmit hw/net/e1000e_core.c:934 #12 0x5571b8fbe011 in e1000e_set_tctl hw/net/e1000e_core.c:2431 #13 0x5571b8fc01e0 in e1000e_core_write hw/net/e1000e_core.c:3265 #14 0x5571b8f9c387 in e1000e_mmio_write hw/net/e1000e.c:109 ... previously allocated by thread T0 here: #0 0x7f89e3b87667 in __interceptor_malloc (/lib64/libasan.so.6+0xb0667) #1 0x7f89e37c7978 in g_malloc (/lib64/libglib-2.0.so.0+0x58978) #2 0x5571b8f95fc7 in net_rx_pkt_pull_data hw/net/net_rx_pkt.c:103 #3 0x5571b8f969b3 in net_rx_pkt_attach_iovec_ex hw/net/net_rx_pkt.c:158 #4 0x5571b8fba6bc in e1000e_receive_iov hw/net/e1000e_core.c:1695 #5 0x5571b8f9cb08 in e1000e_nc_receive_iov hw/net/e1000e.c:213 #6 0x5571b8f90285 in net_tx_pkt_sendv hw/net/net_tx_pkt.c:544 #7 0x5571b8f90aaa in net_tx_pkt_send hw/net/net_tx_pkt.c:620 #8 0x5571b8f90b2e in net_tx_pkt_send_loopback hw/net/net_tx_pkt.c:633 #9 0x5571b8fb3c3b in e1000e_tx_pkt_send hw/net/e1000e_core.c:664 #10 0x5571b8fb43dd in e1000e_process_tx_desc hw/net/e1000e_core.c:743 #11 0x5571b8fb5b4f in e1000e_start_xmit hw/net/e1000e_core.c:934 #12 0x5571b8fbe2ad in e1000e_set_tdt hw/net/e1000e_core.c:2451 #13 0x5571b8fc01e0 in e1000e_core_write hw/net/e1000e_core.c:3265 #14 0x5571b8f9c387 in e1000e_mmio_write hw/net/e1000e.c:109 === | >>> --- a/hw/net/net_tx_pkt.c | >>> +++ b/hw/net/net_tx_pkt.c | >>> @@ -607,12 +607,10 @@ bool net_tx_pkt_send(struct NetTxPkt *pkt, | >>> NetClientState *nc) | >>> * Since underlying infrastructure does not support IP datagrams | >>> longer | >>> * than 64K we should drop such packets and don't even try to send | >>> */ | >>> - if (VIRTIO_NET_HDR_GSO_NONE != pkt->virt_hdr.gso_type) { | >>> - if (pkt->payload_len > | >>> - ETH_MAX_IP_DGRAM_LEN - | >>> - pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len) { | >>> - return false; | >>> - } | >>> + if (pkt->payload_len > | >>> + ETH_MAX_IP_DGRAM_LEN - | >>> + pkt->vec[NET_TX_PKT_L3HDR_FRAG].iov_len) { | >>> + return false; | >>> } Nevertheless, checking 'payload_len' for all packets irrespective of the 'gso_type' does seem reasonable. Thank you. -- Prasad J Pandit / Red Hat Product Security Team 8685 545E B54C 486B C6EB 271E E285 8B5A F050 DE8D