Skip to content

Commit 1328fe0

Browse files
Prasad J Panditkraxel
Prasad J Pandit
authored andcommitted
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 <[email protected]> Reported-by: Yongkang Jia <[email protected]> Reported-by: Yi Ren <[email protected]> Signed-off-by: Prasad J Pandit <[email protected]> Message-id: [email protected] Signed-off-by: Gerd Hoffmann <[email protected]>
1 parent 26d56f4 commit 1328fe0

File tree

1 file changed

+22
-2
lines changed

1 file changed

+22
-2
lines changed

hw/usb/hcd-ohci.c

+22-2
Original file line numberDiff line numberDiff line change
@@ -731,7 +731,11 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
731731
}
732732

733733
start_offset = iso_td.offset[relative_frame_number];
734-
next_offset = iso_td.offset[relative_frame_number + 1];
734+
if (relative_frame_number < frame_count) {
735+
next_offset = iso_td.offset[relative_frame_number + 1];
736+
} else {
737+
next_offset = iso_td.be;
738+
}
735739

736740
if (!(OHCI_BM(start_offset, TD_PSW_CC) & 0xe) ||
737741
((relative_frame_number < frame_count) &&
@@ -764,7 +768,12 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
764768
}
765769
} else {
766770
/* Last packet in the ISO TD */
767-
end_addr = iso_td.be;
771+
end_addr = next_offset;
772+
}
773+
774+
if (start_addr > end_addr) {
775+
trace_usb_ohci_iso_td_bad_cc_overrun(start_addr, end_addr);
776+
return 1;
768777
}
769778

770779
if ((start_addr & OHCI_PAGE_MASK) != (end_addr & OHCI_PAGE_MASK)) {
@@ -773,6 +782,9 @@ static int ohci_service_iso_td(OHCIState *ohci, struct ohci_ed *ed,
773782
} else {
774783
len = end_addr - start_addr + 1;
775784
}
785+
if (len > sizeof(ohci->usb_buf)) {
786+
len = sizeof(ohci->usb_buf);
787+
}
776788

777789
if (len && dir != OHCI_TD_DIR_IN) {
778790
if (ohci_copy_iso_td(ohci, start_addr, end_addr, ohci->usb_buf, len,
@@ -975,8 +987,16 @@ static int ohci_service_td(OHCIState *ohci, struct ohci_ed *ed)
975987
if ((td.cbp & 0xfffff000) != (td.be & 0xfffff000)) {
976988
len = (td.be & 0xfff) + 0x1001 - (td.cbp & 0xfff);
977989
} else {
990+
if (td.cbp > td.be) {
991+
trace_usb_ohci_iso_td_bad_cc_overrun(td.cbp, td.be);
992+
ohci_die(ohci);
993+
return 1;
994+
}
978995
len = (td.be - td.cbp) + 1;
979996
}
997+
if (len > sizeof(ohci->usb_buf)) {
998+
len = sizeof(ohci->usb_buf);
999+
}
9801000

9811001
pktlen = len;
9821002
if (len && dir != OHCI_TD_DIR_IN) {

0 commit comments

Comments
 (0)