[PULL 0/2] Two small fixes for QEMU 7.1-rc3

[PULL 0/2] Two small fixes for QEMU 7.1-rc3

[Thomas Huth]

 Hi Richard!

Two minor fixes for rc3. If this is too late for rc3, please feel free
to ignore, I think they are not severe enough to justify an rc4 later.

The following changes since commit d102b8162a1e5fe8288d4d5c01801ce6536ac2d1:

  Merge tag 'pull-la-20220814' of https://gitlab.com/rth7680/qemu into staging (2022-08-14 08:48:11 -0500)

are available in the Git repository at:

  https://gitlab.com/thuth/qemu.git tags/pull-request-2022-08-16

for you to fetch changes up to effaf5a240e03020f4ae953e10b764622c3e87cc:

  hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394) (2022-08-16 11:37:19 +0200)

----------------------------------------------------------------
* Fix a possible endless loop in USB XHCI code
* Minor fixes for the new readconfig test

----------------------------------------------------------------
Daniel P. Berrangé (1):
      tests/qtest: misc tweaks to readconfig

Thomas Huth (1):
      hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394)

 hw/usb/hcd-xhci.c             | 23 +++++++++++++++++++----
 tests/qtest/readconfig-test.c | 12 ++++++------
 2 files changed, 25 insertions(+), 10 deletions(-)

[PULL 1/2] tests/qtest: misc tweaks to readconfig

[Thomas Huth]

From: Daniel P. Berrangé <berrange@redhat.com>

The property name parameter is ignored when visiting a top
level type, but the obvious typo should be fixed to avoid
confusion. A few indentation issues were tidied up. We
can break out of the loop when finding the RNG device.
Finally, close the temp FD immediately when no longer
needed.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
Message-Id: <20220809093854.168438-1-berrange@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Thomas Huth <thuth@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 tests/qtest/readconfig-test.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/tests/qtest/readconfig-test.c b/tests/qtest/readconfig-test.c
index 2e604d7c2d..c7a9b0c7dd 100644
--- a/tests/qtest/readconfig-test.c
+++ b/tests/qtest/readconfig-test.c
@@ -33,13 +33,12 @@ static QTestState *qtest_init_with_config(const char *cfgdata)
     g_assert_cmpint(cfgfd, >=, 0);
 
     ret = qemu_write_full(cfgfd, cfgdata, strlen(cfgdata));
+    close(cfgfd);
     if (ret < 0) {
         unlink(cfgpath);
     }
     g_assert_cmpint(ret, ==, strlen(cfgdata));
 
-    close(cfgfd);
-
     args = g_strdup_printf("-nodefaults -machine none -readconfig %s", cfgpath);
 
     qts = qtest_init(args);
@@ -79,7 +78,7 @@ static void test_x86_memdev(void)
         "size = \"200\"";
 
     qts = qtest_init_with_config(cfgdata);
-   /* Test valid command */
+    /* Test valid command */
     resp = qtest_qmp(qts, "{ 'execute': 'query-memdev' }");
     test_x86_memdev_resp(qdict_get(resp, "return"));
     qobject_unref(resp);
@@ -96,7 +95,7 @@ static void test_spice_resp(QObject *res)
 
     g_assert(res);
     v = qobject_input_visitor_new(res);
-    visit_type_SpiceInfo(v, "spcie", &spice, &error_abort);
+    visit_type_SpiceInfo(v, "spice", &spice, &error_abort);
 
     g_assert(spice);
     g_assert(spice->enabled);
@@ -114,7 +113,7 @@ static void test_spice(void)
         "unix = \"on\"\n";
 
     qts = qtest_init_with_config(cfgdata);
-   /* Test valid command */
+    /* Test valid command */
     resp = qtest_qmp(qts, "{ 'execute': 'query-spice' }");
     test_spice_resp(qdict_get(resp, "return"));
     qobject_unref(resp);
@@ -144,6 +143,7 @@ static void test_object_rng_resp(QObject *res)
         if (g_str_equal(obj->name, "rng0") &&
             g_str_equal(obj->type, "child<rng-builtin>")) {
             seen_rng = true;
+            break;
         }
 
         tmp = tmp->next;
@@ -164,7 +164,7 @@ static void test_object_rng(void)
         "id = \"rng0\"\n";
 
     qts = qtest_init_with_config(cfgdata);
-   /* Test valid command */
+    /* Test valid command */
     resp = qtest_qmp(qts,
                      "{ 'execute': 'qom-list',"
                      "  'arguments': {'path': '/objects' }}");
-- 
2.31.1

[PULL 2/2] hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394)

[Thomas Huth]

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
Message-Id: <20220804131300.96368-1-thuth@redhat.com>
Reviewed-by: Mauro Matteo Cascella <mcascell@redhat.com>
Acked-by: Gerd Hoffmann <kraxel@redhat.com>
Signed-off-by: Thomas Huth <thuth@redhat.com>
---
 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 296cc6c8e6..3c48b58dde 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"
@@ -725,10 +726,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);
@@ -762,7 +767,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

Re: [PULL 0/2] Two small fixes for QEMU 7.1-rc3

[Richard Henderson]

On 8/16/22 04:58, Thomas Huth wrote:
>   Hi Richard!
> 
> Two minor fixes for rc3. If this is too late for rc3, please feel free
> to ignore, I think they are not severe enough to justify an rc4 later.
> 
> The following changes since commit d102b8162a1e5fe8288d4d5c01801ce6536ac2d1:
> 
>    Merge tag 'pull-la-20220814' of https://gitlab.com/rth7680/qemu into staging (2022-08-14 08:48:11 -0500)
> 
> are available in the Git repository at:
> 
>    https://gitlab.com/thuth/qemu.git tags/pull-request-2022-08-16
> 
> for you to fetch changes up to effaf5a240e03020f4ae953e10b764622c3e87cc:
> 
>    hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394) (2022-08-16 11:37:19 +0200)
> 
> ----------------------------------------------------------------
> * Fix a possible endless loop in USB XHCI code
> * Minor fixes for the new readconfig test

Applied, thanks.  Please update https://wiki.qemu.org/ChangeLog/7.1 as appropriate.


r~


> 
> ----------------------------------------------------------------
> Daniel P. Berrangé (1):
>        tests/qtest: misc tweaks to readconfig
> 
> Thomas Huth (1):
>        hw/usb/hcd-xhci: Fix unbounded loop in xhci_ring_chain_length() (CVE-2020-14394)
> 
>   hw/usb/hcd-xhci.c             | 23 +++++++++++++++++++----
>   tests/qtest/readconfig-test.c | 12 ++++++------
>   2 files changed, 25 insertions(+), 10 deletions(-)
>