1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
| | Fix CVE-2018-16847:
https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-16847
Patch copied from upstream source repository:
https://git.qemu.org/?p=qemu.git;a=commitdiff;h=87ad860c622cc8f8916b5232bd8728c08f938fce
From 87ad860c622cc8f8916b5232bd8728c08f938fce Mon Sep 17 00:00:00 2001
From: Paolo Bonzini <pbonzini@redhat.com>
Date: Tue, 20 Nov 2018 19:41:48 +0100
Subject: [PATCH] nvme: fix out-of-bounds access to the CMB
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
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@intel.com>
Cc: qemu-block@nongnu.org
Reported-by: Li Qiang <liq3ea@gmail.com>
Reviewed-by: Li Qiang <liq3ea@gmail.com>
Tested-by: Li Qiang <liq3ea@gmail.com>
Signed-off-by: Paolo Bonzini <pbonzini@redhat.com>
Reviewed-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Tested-by: Philippe Mathieu-Daudé <philmd@redhat.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
---
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 28d284346dd..8c35cab2b43 100644
--- a/hw/block/nvme.c
+++ b/hw/block/nvme.c
@@ -1201,7 +1201,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 613242bc6ef..fb0b449c02a 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 7674a446e4f..2700ba838aa 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();
}
--
2.19.2
|