From: Thomas Huth <thuth@redhat.com>
To: Gerd Hoffmann <kraxel@redhat.com>, qemu-devel@nongnu.org
Cc: mcascell@redhat.com, f4bug@amsat.org,
Peter Maydell <peter.maydell@linaro.org>
Subject: [PATCH v2 for-7.1] hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394)
Date: Thu, 4 Aug 2022 15:13:00 +0200 [thread overview]
Message-ID: <20220804131300.96368-1-thuth@redhat.com> (raw)
The loop condition in xhci_ring_chain_length() is under control of
the guest, and additionally the code does not check for failed DMA
transfers (e.g. if reaching the end of the RAM), so the loop there
could run for a very long time or even forever. Fix it by checking
the return value of dma_memory_read() and by introducing a maximum
loop length.
Resolves: https://gitlab.com/qemu-project/qemu/-/issues/646
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
v2:
- Reworded subject and commit description
- Focus on xhci_ring_chain_length() since that's the only function
where an endless loop can currently occur. The other spots that
ignore the return value of dma_memory_read() should be fixed as
well later, but that's rather something for QEMU 7.2.
- Added an real limit for the loop, so that it also ends after a
while in case there are no DMA errors
- Use "return -1" instead of "return -length" since the latter
is somewhat weird (could be sometimes 0, sometimes negative)
hw/usb/hcd-xhci.c | 23 +++++++++++++++++++----
1 file changed, 19 insertions(+), 4 deletions(-)
diff --git a/hw/usb/hcd-xhci.c b/hw/usb/hcd-xhci.c
index 5a1ddf8c3e..b5669bc234 100644
--- a/hw/usb/hcd-xhci.c
+++ b/hw/usb/hcd-xhci.c
@@ -21,6 +21,7 @@
#include "qemu/osdep.h"
#include "qemu/timer.h"
+#include "qemu/log.h"
#include "qemu/module.h"
#include "qemu/queue.h"
#include "migration/vmstate.h"
@@ -729,10 +730,14 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
bool control_td_set = 0;
uint32_t link_cnt = 0;
- while (1) {
+ do {
TRBType type;
- dma_memory_read(xhci->as, dequeue, &trb, TRB_SIZE,
- MEMTXATTRS_UNSPECIFIED);
+ if (dma_memory_read(xhci->as, dequeue, &trb, TRB_SIZE,
+ MEMTXATTRS_UNSPECIFIED) != MEMTX_OK) {
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: DMA memory access failed!\n",
+ __func__);
+ return -1;
+ }
le64_to_cpus(&trb.parameter);
le32_to_cpus(&trb.status);
le32_to_cpus(&trb.control);
@@ -766,7 +771,17 @@ static int xhci_ring_chain_length(XHCIState *xhci, const XHCIRing *ring)
if (!control_td_set && !(trb.control & TRB_TR_CH)) {
return length;
}
- }
+
+ /*
+ * According to the xHCI spec, Transfer Ring segments should have
+ * a maximum size of 64 kB (see chapter "6 Data Structures")
+ */
+ } while (length < TRB_LINK_LIMIT * 65536 / TRB_SIZE);
+
+ qemu_log_mask(LOG_GUEST_ERROR, "%s: exceeded maximum tranfer ring size!\n",
+ __func__);
+
+ return -1;
}
static void xhci_er_reset(XHCIState *xhci, int v)
--
2.31.1
next reply other threads:[~2022-08-04 13:24 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-08-04 13:13 Thomas Huth [this message]
2022-08-05 9:22 ` [PATCH v2 for-7.1] hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394) Mauro Matteo Cascella
2022-08-16 9:02 ` Gerd Hoffmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20220804131300.96368-1-thuth@redhat.com \
--to=thuth@redhat.com \
--cc=f4bug@amsat.org \
--cc=kraxel@redhat.com \
--cc=mcascell@redhat.com \
--cc=peter.maydell@linaro.org \
--cc=qemu-devel@nongnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).