* [PULL 1/3] usb: fix u2f build
2020-09-21 8:58 [PULL 0/3] Usb 20200921 patches Gerd Hoffmann
@ 2020-09-21 8:58 ` Gerd Hoffmann
2020-09-21 8:58 ` [PULL 2/3] hw: usb: hcd-ohci: check len and frame_number variables Gerd Hoffmann
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Gerd Hoffmann @ 2020-09-21 8:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Daniel P . Berrangé, Gerd Hoffmann, César Belley
Just use qemu_open_old() for a quick fix, switch
to better error handling left for another day.
Fixes: 448058aa99aa ("util: rename qemu_open() to qemu_open_old()")
Cc: César Belley <cesar.belley@lse.epita.fr>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
Message-id: 20200918110122.9121-1-kraxel@redhat.com
---
hw/usb/u2f-emulated.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/hw/usb/u2f-emulated.c b/hw/usb/u2f-emulated.c
index 9e1b829f3d32..9151feb63d44 100644
--- a/hw/usb/u2f-emulated.c
+++ b/hw/usb/u2f-emulated.c
@@ -200,7 +200,7 @@ static ssize_t u2f_emulated_read(const char *path, char *buffer,
int fd;
ssize_t ret;
- fd = qemu_open(path, O_RDONLY);
+ fd = qemu_open_old(path, O_RDONLY);
if (fd < 0) {
return -1;
}
@@ -217,7 +217,7 @@ static bool u2f_emulated_setup_counter(const char *path,
int fd, ret;
FILE *fp;
- fd = qemu_open(path, O_RDWR);
+ fd = qemu_open_old(path, O_RDWR);
if (fd < 0) {
return false;
}
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PULL 2/3] hw: usb: hcd-ohci: check len and frame_number variables
2020-09-21 8:58 [PULL 0/3] Usb 20200921 patches Gerd Hoffmann
2020-09-21 8:58 ` [PULL 1/3] usb: fix u2f build Gerd Hoffmann
@ 2020-09-21 8:58 ` Gerd Hoffmann
2020-09-21 8:58 ` [PULL 3/3] hw: usb: hcd-ohci: check for processed TD before retire Gerd Hoffmann
2020-09-22 17:19 ` [PULL 0/3] Usb 20200921 patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Gerd Hoffmann @ 2020-09-21 8:58 UTC (permalink / raw)
To: qemu-devel
Cc: Yi Ren, Yongkang Jia, Gaoning Pan, Gerd Hoffmann, Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
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 <pgn@zju.edu.cn>
Reported-by: Yongkang Jia <j_kangel@163.com>
Reported-by: Yi Ren <yunye.ry@alibaba-inc.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-id: 20200915182259.68522-2-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
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 1e6e85e86a8d..9dc59101f958 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -731,7 +731,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) &&
@@ -764,7 +768,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)) {
@@ -773,6 +782,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,
@@ -975,8 +987,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) {
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PULL 3/3] hw: usb: hcd-ohci: check for processed TD before retire
2020-09-21 8:58 [PULL 0/3] Usb 20200921 patches Gerd Hoffmann
2020-09-21 8:58 ` [PULL 1/3] usb: fix u2f build Gerd Hoffmann
2020-09-21 8:58 ` [PULL 2/3] hw: usb: hcd-ohci: check len and frame_number variables Gerd Hoffmann
@ 2020-09-21 8:58 ` Gerd Hoffmann
2020-09-22 17:19 ` [PULL 0/3] Usb 20200921 patches Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Gerd Hoffmann @ 2020-09-21 8:58 UTC (permalink / raw)
To: qemu-devel; +Cc: Li Qiang, Gerd Hoffmann, Prasad J Pandit
From: Prasad J Pandit <pjp@fedoraproject.org>
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 <pjp@fedoraproject.org>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Message-id: 20200915182259.68522-3-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
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 9dc59101f958..8b912e95d31d 100644
--- a/hw/usb/hcd-ohci.c
+++ b/hw/usb/hcd-ohci.c
@@ -691,6 +691,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);
--
2.27.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PULL 0/3] Usb 20200921 patches
2020-09-21 8:58 [PULL 0/3] Usb 20200921 patches Gerd Hoffmann
` (2 preceding siblings ...)
2020-09-21 8:58 ` [PULL 3/3] hw: usb: hcd-ohci: check for processed TD before retire Gerd Hoffmann
@ 2020-09-22 17:19 ` Peter Maydell
3 siblings, 0 replies; 5+ messages in thread
From: Peter Maydell @ 2020-09-22 17:19 UTC (permalink / raw)
To: Gerd Hoffmann; +Cc: QEMU Developers
On Mon, 21 Sep 2020 at 10:00, Gerd Hoffmann <kraxel@redhat.com> wrote:
>
> The following changes since commit 053a4177817db307ec854356e95b5b350800a216:
>
> Merge remote-tracking branch 'remotes/philmd-gitlab/tags/fw_cfg-20200918' into staging (2020-09-18 16:34:26 +0100)
>
> are available in the Git repository at:
>
> git://git.kraxel.org/qemu tags/usb-20200921-pull-request
>
> for you to fetch changes up to 1be90ebecc95b09a2ee5af3f60c412b45a766c4f:
>
> hw: usb: hcd-ohci: check for processed TD before retire (2020-09-21 09:44:54 +0200)
>
> ----------------------------------------------------------------
> usb: fix u2f build
> usb: fix ohci oob access and loop issues
>
> ----------------------------------------------------------------
Applied, thanks.
Please update the changelog at https://wiki.qemu.org/ChangeLog/5.2
for any user-visible changes.
-- PMM
^ permalink raw reply [flat|nested] 5+ messages in thread