From 3cd5fd3c0c03b6c852586470e955334b6c4760b9 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Tue, 15 Sep 2020 23:52:59 +0530 Subject: [PATCH 1/4] hw: usb: hcd-ohci: check for processed TD before retire While servicing OHCI transfer descriptors(TD), ohci_service_iso_td retires a TD if it has passed its time frame. It does not check if the TD was already processed once and holds an error code in TD_CC. It may happen if the TD list has a loop. Add check to avoid an infinite loop condition. Signed-off-by: Prasad J Pandit Reviewed-by: Li Qiang Message-id: 20200915182259.68522-3-ppandit@redhat.com Signed-off-by: Gerd Hoffmann (cherry-picked from 1be90ebe) Fix CVE-2020-25625 Signed-off-by: Alex Chen --- hw/usb/hcd-ohci.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index 4f6fdbc0a7..ffe52a09d7 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -689,6 +689,10 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, the next ISO TD of the same ED */ trace_usb_ohci_iso_td_relative_frame_number_big(relative_frame_number, frame_count); + if (OHCI_CC_DATAOVERRUN == OHCI_BM(iso_td.flags, TD_CC)) { + /* avoid infinite loop */ + return 1; + } OHCI_SET_BM(iso_td.flags, TD_CC, OHCI_CC_DATAOVERRUN); ed->head &= ~OHCI_DPTR_MASK; ed->head |= (iso_td.next & OHCI_DPTR_MASK); -- Gitee From e80eae822b8ed765e0ae08e18435389e46357fbd Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Wed, 12 Aug 2020 09:17:27 -0700 Subject: [PATCH 2/4] hw: ehci: check return value of 'usb_packet_map' If 'usb_packet_map' fails, we should stop to process the usb request. Signed-off-by: Li Qiang Message-Id: <20200812161727.29412-1-liq3ea@163.com> Signed-off-by: Gerd Hoffmann (cherry-picked from 2fdb42d8) Fix CVE-2020-25723 Signed-off-by: Alex Chen --- hw/usb/hcd-ehci.c | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-ehci.c b/hw/usb/hcd-ehci.c index 5f089f3005..433e6a4fc0 100644 --- a/hw/usb/hcd-ehci.c +++ b/hw/usb/hcd-ehci.c @@ -1370,7 +1370,10 @@ static int ehci_execute(EHCIPacket *p, const char *action) spd = (p->pid == USB_TOKEN_IN && NLPTR_TBIT(p->qtd.altnext) == 0); usb_packet_setup(&p->packet, p->pid, ep, 0, p->qtdaddr, spd, (p->qtd.token & QTD_TOKEN_IOC) != 0); - usb_packet_map(&p->packet, &p->sgl); + if (usb_packet_map(&p->packet, &p->sgl)) { + qemu_sglist_destroy(&p->sgl); + return -1; + } p->async = EHCI_ASYNC_INITIALIZED; } @@ -1449,7 +1452,10 @@ static int ehci_process_itd(EHCIState *ehci, if (ep && ep->type == USB_ENDPOINT_XFER_ISOC) { usb_packet_setup(&ehci->ipacket, pid, ep, 0, addr, false, (itd->transact[i] & ITD_XACT_IOC) != 0); - usb_packet_map(&ehci->ipacket, &ehci->isgl); + if (usb_packet_map(&ehci->ipacket, &ehci->isgl)) { + qemu_sglist_destroy(&ehci->isgl); + return -1; + } usb_handle_packet(dev, &ehci->ipacket); usb_packet_unmap(&ehci->ipacket, &ehci->isgl); } else { -- Gitee From e82083e09bc20f3a864d208c7be14efd5ff32f51 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Tue, 15 Sep 2020 23:52:58 +0530 Subject: [PATCH 3/4] hw: usb: hcd-ohci: check len and frame_number variables While servicing the OHCI transfer descriptors(TD), OHCI host controller derives variables 'start_addr', 'end_addr', 'len' etc. from values supplied by the host controller driver. Host controller driver may supply values such that using above variables leads to out-of-bounds access issues. Add checks to avoid them. AddressSanitizer: stack-buffer-overflow on address 0x7ffd53af76a0 READ of size 2 at 0x7ffd53af76a0 thread T0 #0 ohci_service_iso_td ../hw/usb/hcd-ohci.c:734 #1 ohci_service_ed_list ../hw/usb/hcd-ohci.c:1180 #2 ohci_process_lists ../hw/usb/hcd-ohci.c:1214 #3 ohci_frame_boundary ../hw/usb/hcd-ohci.c:1257 #4 timerlist_run_timers ../util/qemu-timer.c:572 #5 qemu_clock_run_timers ../util/qemu-timer.c:586 #6 qemu_clock_run_all_timers ../util/qemu-timer.c:672 #7 main_loop_wait ../util/main-loop.c:527 #8 qemu_main_loop ../softmmu/vl.c:1676 #9 main ../softmmu/main.c:50 Reported-by: Gaoning Pan Reported-by: Yongkang Jia Reported-by: Yi Ren Signed-off-by: Prasad J Pandit Message-id: 20200915182259.68522-2-ppandit@redhat.com Signed-off-by: Gerd Hoffmann (cherry-picked from 1328fe0c) Fix CVE-2020-25624 Signed-off-by: Alex Chen --- hw/usb/hcd-ohci.c | 24 ++++++++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/hw/usb/hcd-ohci.c b/hw/usb/hcd-ohci.c index ffe52a09d7..d2dd8efd58 100644 --- a/hw/usb/hcd-ohci.c +++ b/hw/usb/hcd-ohci.c @@ -733,7 +733,11 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, } start_offset = iso_td.offset[relative_frame_number]; - next_offset = iso_td.offset[relative_frame_number + 1]; + if (relative_frame_number < frame_count) { + next_offset = iso_td.offset[relative_frame_number + 1]; + } else { + next_offset = iso_td.be; + } if (!(OHCI_BM(start_offset, TD_PSW_CC) & 0xe) || ((relative_frame_number < frame_count) && @@ -766,7 +770,12 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, } } else { /* Last packet in the ISO TD */ - end_addr = iso_td.be; + end_addr = next_offset; + } + + if (start_addr > end_addr) { + trace_usb_ohci_iso_td_bad_cc_overrun(start_addr, end_addr); + return 1; } if ((start_addr & OHCI_PAGE_MASK) != (end_addr & OHCI_PAGE_MASK)) { @@ -775,6 +784,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed, } else { len = end_addr - start_addr + 1; } + if (len > sizeof(ohci->usb_buf)) { + len = sizeof(ohci->usb_buf); + } if (len && dir != OHCI_TD_DIR_IN) { if (ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len, @@ -977,8 +989,16 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed) if ((td.cbp & 0xfffff000) != (td.be & 0xfffff000)) { len = (td.be & 0xfff) + 0x1001 - (td.cbp & 0xfff); } else { + if (td.cbp > td.be) { + trace_usb_ohci_iso_td_bad_cc_overrun(td.cbp, td.be); + ohci_die(ohci); + return 1; + } len = (td.be - td.cbp) + 1; } + if (len > sizeof(ohci->usb_buf)) { + len = sizeof(ohci->usb_buf); + } pktlen = len; if (len && dir != OHCI_TD_DIR_IN) { -- Gitee From 745e9fea5485e564414a904ce0831bc79c11ac57 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Wed, 11 Nov 2020 18:36:36 +0530 Subject: [PATCH 4/4] hw/net/e1000e: advance desc_offset in case of null descriptor While receiving packets via e1000e_write_packet_to_guest() routine, 'desc_offset' is advanced only when RX descriptor is processed. And RX descriptor is not processed if it has NULL buffer address. This may lead to an infinite loop condition. Increament 'desc_offset' to process next descriptor in the ring to avoid infinite loop. Reported-by: Cheol-woo Myung <330cjfdn@gmail.com> Signed-off-by: Prasad J Pandit Signed-off-by: Jason Wang (cherry-picked from c2cb5116) Fix CVE-2020-28916 Signed-off-by: Alex Chen --- hw/net/e1000e_core.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/hw/net/e1000e_core.c b/hw/net/e1000e_core.c index 2a221c2ef9..e45d47f584 100644 --- a/hw/net/e1000e_core.c +++ b/hw/net/e1000e_core.c @@ -1595,13 +1595,13 @@ e1000e_write_packet_to_guest(E1000ECore *core, struct NetRxPkt *pkt, (const char *) &fcs_pad, e1000x_fcs_len(core->mac)); } } - desc_offset += desc_size; - if (desc_offset >= total_size) { - is_last = true; - } } else { /* as per intel docs; skip descriptors with null buf addr */ trace_e1000e_rx_null_descriptor(); } + desc_offset += desc_size; + if (desc_offset >= total_size) { + is_last = true; + } e1000e_write_rx_descr(core, desc, is_last ? core->rx_pkt : NULL, rss_info, do_ps ? ps_hdr_len : 0, &bastate.written); -- Gitee