[Xen-changelog] [qemu-xen staging-4.12] nvme: fix out-of-bounds access to the CMB

[patchbot@xxxxxxx]

commit 310f2140e78d3e2224a052d57551cd1952f7142e
Author:     Paolo Bonzini <pbonzini@xxxxxxxxxx>
AuthorDate: Tue Nov 20 19:41:48 2018 +0100
Commit:     Anthony PERARD <anthony.perard@xxxxxxxxxx>
CommitDate: Tue Apr 2 17:27:33 2019 +0100

    nvme: fix out-of-bounds access to the CMB
    
    Because the CMB BAR has a min_access_size of 2, if you read the last
    byte it will try to memcpy *2* bytes from n->cmbuf, causing an off-by-one
    error.  This is CVE-2018-16847.
    
    Another way to fix this might be to register the CMB as a RAM memory
    region, which would also be more efficient.  However, that might be a
    change for big-endian machines; I didn't think this through and I don't
    know how real hardware works.  Add a basic testcase for the CMB in case
    somebody does this change later on.
    
    Cc: Keith Busch <keith.busch@xxxxxxxxx>
    Cc: qemu-block@xxxxxxxxxx
    Reported-by: Li Qiang <liq3ea@xxxxxxxxx>
    Reviewed-by: Li Qiang <liq3ea@xxxxxxxxx>
    Tested-by: Li Qiang <liq3ea@xxxxxxxxx>
    Signed-off-by: Paolo Bonzini <pbonzini@xxxxxxxxxx>
    Reviewed-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
    Tested-by: Philippe Mathieu-Daudé <philmd@xxxxxxxxxx>
    Signed-off-by: Kevin Wolf <kwolf@xxxxxxxxxx>
    (cherry picked from commit 87ad860c622cc8f8916b5232bd8728c08f938fce)
---
 hw/block/nvme.c        |  2 +-
 tests/Makefile.include |  2 +-
 tests/nvme-test.c      | 68 ++++++++++++++++++++++++++++++++++++++++++--------
 3 files changed, 60 insertions(+), 12 deletions(-)

diff --git a/hw/block/nvme.c b/hw/block/nvme.c
index 5e508ab1b3..5f304bd098 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1192,7 +1192,7 @@ static const MemoryRegionOps nvme_cmb_ops = {
     .write = nvme_cmb_write,
     .endianness = DEVICE_LITTLE_ENDIAN,
     .impl = {
-        .min_access_size = 2,
+        .min_access_size = 1,
         .max_access_size = 8,
     },
 };
diff --git a/tests/Makefile.include b/tests/Makefile.include
index a49282704e..91af385e2f 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -821,7 +821,7 @@ tests/test-hmp$(EXESUF): tests/test-hmp.o
 tests/machine-none-test$(EXESUF): tests/machine-none-test.o
 tests/drive_del-test$(EXESUF): tests/drive_del-test.o $(libqos-virtio-obj-y)
 tests/qdev-monitor-test$(EXESUF): tests/qdev-monitor-test.o $(libqos-pc-obj-y)
-tests/nvme-test$(EXESUF): tests/nvme-test.o
+tests/nvme-test$(EXESUF): tests/nvme-test.o $(libqos-pc-obj-y)
 tests/pvpanic-test$(EXESUF): tests/pvpanic-test.o
 tests/i82801b11-test$(EXESUF): tests/i82801b11-test.o
 tests/ac97-test$(EXESUF): tests/ac97-test.o
diff --git a/tests/nvme-test.c b/tests/nvme-test.c
index 7674a446e4..2700ba838a 100644
--- a/tests/nvme-test.c
+++ b/tests/nvme-test.c
@@ -8,25 +8,73 @@
  */
 
 #include "qemu/osdep.h"
+#include "qemu/units.h"
 #include "libqtest.h"
+#include "libqos/libqos-pc.h"
+
+static QOSState *qnvme_start(const char *extra_opts)
+{
+    QOSState *qs;
+    const char *arch = qtest_get_arch();
+    const char *cmd = "-drive id=drv0,if=none,file=null-co://,format=raw "
+                      "-device nvme,addr=0x4.0,serial=foo,drive=drv0 %s";
+
+    if (strcmp(arch, "i386") == 0 || strcmp(arch, "x86_64") == 0) {
+        qs = qtest_pc_boot(cmd, extra_opts ? : "");
+        global_qtest = qs->qts;
+        return qs;
+    }
+
+    g_printerr("nvme tests are only available on x86\n");
+    exit(EXIT_FAILURE);
+}
+
+static void qnvme_stop(QOSState *qs)
+{
+    qtest_shutdown(qs);
+}
 
-/* Tests only initialization so far. TODO: Replace with functional tests */
 static void nop(void)
 {
+    QOSState *qs;
+
+    qs = qnvme_start(NULL);
+    qnvme_stop(qs);
 }
 
-int main(int argc, char **argv)
+static void nvmetest_cmb_test(void)
 {
-    int ret;
+    const int cmb_bar_size = 2 * MiB;
+    QOSState *qs;
+    QPCIDevice *pdev;
+    QPCIBar bar;
 
-    g_test_init(&argc, &argv, NULL);
-    qtest_add_func("/nvme/nop", nop);
+    qs = qnvme_start("-global nvme.cmb_size_mb=2");
+    pdev = qpci_device_find(qs->pcibus, QPCI_DEVFN(4,0));
+    g_assert(pdev != NULL);
+
+    qpci_device_enable(pdev);
+    bar = qpci_iomap(pdev, 2, NULL);
+
+    qpci_io_writel(pdev, bar, 0, 0xccbbaa99);
+    g_assert_cmpint(qpci_io_readb(pdev, bar, 0), ==, 0x99);
+    g_assert_cmpint(qpci_io_readw(pdev, bar, 0), ==, 0xaa99);
+
+    /* Test partially out-of-bounds accesses.  */
+    qpci_io_writel(pdev, bar, cmb_bar_size - 1, 0x44332211);
+    g_assert_cmpint(qpci_io_readb(pdev, bar, cmb_bar_size - 1), ==, 0x11);
+    g_assert_cmpint(qpci_io_readw(pdev, bar, cmb_bar_size - 1), !=, 0x2211);
+    g_assert_cmpint(qpci_io_readl(pdev, bar, cmb_bar_size - 1), !=, 
0x44332211);
+    g_free(pdev);
 
-    qtest_start("-drive id=drv0,if=none,file=null-co://,format=raw "
-                "-device nvme,drive=drv0,serial=foo");
-    ret = g_test_run();
+    qnvme_stop(qs);
+}
 
-    qtest_end();
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    qtest_add_func("/nvme/nop", nop);
+    qtest_add_func("/nvme/cmb_test", nvmetest_cmb_test);
 
-    return ret;
+    return g_test_run();
 }
--
generated by git-patchbot for /home/xen/git/qemu-xen.git#staging-4.12

_______________________________________________
Xen-changelog mailing list
Xen-changelog@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/xen-changelog