On 20/11/18 19:41, Paolo Bonzini wrote:
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
Cc: qemu-block@nongnu.org
Reported-by: Li Qiang
Reviewed-by: Li Qiang
Tested-by: Li Qiang
Signed-off-by: Paolo Bonzini
---
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 d0226e7fdc..ef046bbc54 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1199,7 +1199,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 613242bc6e..fb0b449c02 100644
--- a/tests/Makefile.include
+++ b/tests/Makefile.include
@@ -730,7 +730,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();
}
TEST: tests/nvme-test... (pid=29324)
/x86_64/nvme/nop: OK
/x86_64/nvme/cmb_test: **
ERROR:tests/nvme-test.c:65:nvmetest_cmb_test: assertion failed
(qpci_io_readb(pdev, bar, cmb_bar_size - 1) == 0x11): (0 == 17)
FAIL
Nice!
Reviewed-by: Philippe Mathieu-Daudé
Tested-by: Philippe Mathieu-Daudé