| Message ID | 5FA405FE.1080604@huawei.com |
|---|---|
| State | New |
| Headers | show |
| Series | usb/hcd-xhci: Fix an index-out-of-bounds in xhci_runtime_write and xhci_runtime_read | expand |
Kindly ping... On 2020/11/5 22:02, AlexChen wrote: > Currently, the 'v' is not checked whether it is between 0 and 16, > which may result in an out-of-bounds access to the array 'xhci->intr[]'. > This is LP#1902112. Following is the reproducer provided in: > -->https://bugs.launchpad.net/qemu/+bug/1902112 > > === Reproducer (build with --enable-sanitizers) === > export UBSAN_OPTIONS="print_stacktrace=1:silence_unsigned_overflow=1" > cat << EOF | ./qemu-system-i386 -display none -machine\ > accel=qtest, -m 512M -machine q35 -nodefaults -drive\ > file=null-co://,if=none,format=raw,id=disk0 -device\ > qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0\ > -device usb-bot -device usb-storage,drive=disk0\ > -chardev null,id=cd0 -chardev null,id=cd1 -device\ > usb-braille,chardev=cd0 -device usb-ccid -device\ > usb-ccid -device usb-kbd -device usb-mouse -device\ > usb-serial,chardev=cd1 -device usb-tablet -device\ > usb-wacom-tablet -device usb-audio -qtest stdio > outl 0xcf8 0x80000803 > outl 0xcfc 0x18caffff > outl 0xcf8 0x80000810 > outl 0xcfc 0x555a2e46 > write 0x555a1004 0x4 0xe7b9aa7a > EOF > > === Stack Trace === > > SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../hw/usb/hcd-xhci.c:3012:30 in > ../hw/usb/hcd-xhci.c:3012:30: runtime error: index -1 out of bounds for type 'XHCIInterrupter [16]' > #0 0x55bd2e97c8b0 in xhci_runtime_write /src/qemu/hw/usb/hcd-xhci.c:3012:30 > #1 0x55bd2edfdd13 in memory_region_write_accessor /src/qemu/softmmu/memory.c:484:5 > #2 0x55bd2edfdb14 in access_with_adjusted_size /src/qemu/softmmu/memory.c:545:18 > #3 0x55bd2edfd54b in memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13 > #4 0x55bd2ed7fa46 in flatview_write_continue /src/qemu/softmmu/physmem.c:2767:23 > #5 0x55bd2ed7cac0 in flatview_write /src/qemu/softmmu/physmem.c:2807:14 > > This patch fixes this bug. > > Buglink: https://bugs.launchpad.net/qemu/+bug/1902112 > Reported-by: Alexander Bulekov <alxndr@bu.edu> > Signed-off-by: Alex Chen <alex.chen@huawei.com> > --- > hw/usb/hcd-xhci.c | 10 ++++++---- > 1 file changed, 6 insertions(+), 4 deletions(-) > > diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c > index 79ce5c4be6..50abef40ad 100644 > --- a/hw/usb/hcd-xhci.c > +++ b/hw/usb/hcd-xhci.c > @@ -2962,8 +2962,9 @@ static uint64_t xhci_runtime_read(void *ptr, hwaddr reg, > { > XHCIState *xhci = ptr; > uint32_t ret = 0; > + int v = (reg - 0x20) / 0x20; > > - if (reg < 0x20) { > + if (reg < 0x20 || v < 0 || v >= XHCI_MAXINTRS) { > switch (reg) { > case 0x00: /* MFINDEX */ > ret = xhci_mfindex_get(xhci) & 0x3fff; > @@ -2973,7 +2974,6 @@ static uint64_t xhci_runtime_read(void *ptr, hwaddr reg, > break; > } > } else { > - int v = (reg - 0x20) / 0x20; > XHCIInterrupter *intr = &xhci->intr[v]; > switch (reg & 0x1f) { > case 0x00: /* IMAN */ > @@ -3009,14 +3009,16 @@ static void xhci_runtime_write(void *ptr, hwaddr reg, > { > XHCIState *xhci = ptr; > int v = (reg - 0x20) / 0x20; > - XHCIInterrupter *intr = &xhci->intr[v]; > + XHCIInterrupter *intr; > trace_usb_xhci_runtime_write(reg, val); > > - if (reg < 0x20) { > + if (reg < 0x20 || v < 0 || v >= XHCI_MAXINTRS) { > trace_usb_xhci_unimplemented("runtime write", reg); > return; > } > > + intr = &xhci->intr[v]; > + > switch (reg & 0x1f) { > case 0x00: /* IMAN */ > if (val & IMAN_IP) { >
=== Reproducer (build with --enable-sanitizers) === export UBSAN_OPTIONS="print_stacktrace=1:silence_unsigned_overflow=1" cat << EOF | ./qemu-system-i386 -display none -machine\ accel=qtest, -m 512M -machine q35 -nodefaults -drive\ file=null-co://,if=none,format=raw,id=disk0 -device\ qemu-xhci,id=xhci -device usb-tablet,bus=xhci.0\ -device usb-bot -device usb-storage,drive=disk0\ -chardev null,id=cd0 -chardev null,id=cd1 -device\ usb-braille,chardev=cd0 -device usb-ccid -device\ usb-ccid -device usb-kbd -device usb-mouse -device\ usb-serial,chardev=cd1 -device usb-tablet -device\ usb-wacom-tablet -device usb-audio -qtest stdio outl 0xcf8 0x80000803 outl 0xcfc 0x18caffff outl 0xcf8 0x80000810 outl 0xcfc 0x555a2e46 write 0x555a1004 0x4 0xe7b9aa7a EOF === Stack Trace === SUMMARY: UndefinedBehaviorSanitizer: undefined-behavior ../hw/usb/hcd-xhci.c:3012:30 in ../hw/usb/hcd-xhci.c:3012:30: runtime error: index -1 out of bounds for type 'XHCIInterrupter [16]' #0 0x55bd2e97c8b0 in xhci_runtime_write /src/qemu/hw/usb/hcd-xhci.c:3012:30 #1 0x55bd2edfdd13 in memory_region_write_accessor /src/qemu/softmmu/memory.c:484:5 #2 0x55bd2edfdb14 in access_with_adjusted_size /src/qemu/softmmu/memory.c:545:18 #3 0x55bd2edfd54b in memory_region_dispatch_write /src/qemu/softmmu/memory.c:0:13 #4 0x55bd2ed7fa46 in flatview_write_continue /src/qemu/softmmu/physmem.c:2767:23 #5 0x55bd2ed7cac0 in flatview_write /src/qemu/softmmu/physmem.c:2807:14 This patch fixes this bug. Buglink: https://bugs.launchpad.net/qemu/+bug/1902112 Reported-by: Alexander Bulekov <alxndr@bu.edu> Signed-off-by: Alex Chen <alex.chen@huawei.com> --- hw/usb/hcd-xhci.c | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c index 79ce5c4be6..50abef40ad 100644 --- a/hw/usb/hcd-xhci.c +++ b/hw/usb/hcd-xhci.c @@ -2962,8 +2962,9 @@ static uint64_t xhci_runtime_read(void *ptr, hwaddr reg, { XHCIState *xhci = ptr; uint32_t ret = 0; + int v = (reg - 0x20) / 0x20; - if (reg < 0x20) { + if (reg < 0x20 || v < 0 || v >= XHCI_MAXINTRS) { switch (reg) { case 0x00: /* MFINDEX */ ret = xhci_mfindex_get(xhci) & 0x3fff; @@ -2973,7 +2974,6 @@ static uint64_t xhci_runtime_read(void *ptr, hwaddr reg, break; } } else { - int v = (reg - 0x20) / 0x20; XHCIInterrupter *intr = &xhci->intr[v]; switch (reg & 0x1f) { case 0x00: /* IMAN */ @@ -3009,14 +3009,16 @@ static void xhci_runtime_write(void *ptr, hwaddr reg, { XHCIState *xhci = ptr; int v = (reg - 0x20) / 0x20; - XHCIInterrupter *intr = &xhci->intr[v]; + XHCIInterrupter *intr; trace_usb_xhci_runtime_write(reg, val); - if (reg < 0x20) { + if (reg < 0x20 || v < 0 || v >= XHCI_MAXINTRS) { trace_usb_xhci_unimplemented("runtime write", reg); return; } + intr = &xhci->intr[v]; + switch (reg & 0x1f) { case 0x00: /* IMAN */ if (val & IMAN_IP) {