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
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
|
From: Gerd Hoffmann <kraxel@redhat.com>
Date: Tue, 26 Aug 2014 15:35:23 +0200
Subject: [PATCH] vbe: rework sanity checks
Plug a bunch of holes in the bochs dispi interface parameter checking.
Add a function doing verification on all registers. Call that
unconditionally on every register write. That way we should catch
everything, even changing one register affecting the valid range of
another register.
Some of the holes have been added by commit
e9c6149f6ae6873f14a12eea554925b6aa4c4dec. Before that commit the
maximum possible framebuffer (VBE_DISPI_MAX_XRES * VBE_DISPI_MAX_YRES *
32 bpp) has been smaller than the qemu vga memory (8MB) and the checking
for VBE_DISPI_MAX_XRES + VBE_DISPI_MAX_YRES + VBE_DISPI_MAX_BPP was ok.
Some of the holes have been there forever, such as
VBE_DISPI_INDEX_X_OFFSET and VBE_DISPI_INDEX_Y_OFFSET register writes
lacking any verification.
Security impact:
(1) Guest can make the ui (gtk/vnc/...) use memory rages outside the vga
frame buffer as source -> host memory leak. Memory isn't leaked to
the guest but to the vnc client though.
(2) Qemu will segfault in case the memory range happens to include
unmapped areas -> Guest can DoS itself.
The guest can not modify host memory, so I don't think this can be used
by the guest to escape.
CVE-2014-3615
Cc: qemu-stable@nongnu.org
Cc: secalert@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Laszlo Ersek <lersek@redhat.com>
(cherry picked from commit c1b886c45dc70f247300f549dce9833f3fa2def5)
---
hw/display/vga.c | 154 ++++++++++++++++++++++++++++++++++---------------------
1 file changed, 95 insertions(+), 59 deletions(-)
diff --git a/hw/display/vga.c b/hw/display/vga.c
index 2f13e43..cdb2e6b 100644
--- a/hw/display/vga.c
+++ b/hw/display/vga.c
@@ -580,6 +580,93 @@ void vga_ioport_write(void *opaque, uint32_t addr, uint32_t val)
}
}
+/*
+ * Sanity check vbe register writes.
+ *
+ * As we don't have a way to signal errors to the guest in the bochs
+ * dispi interface we'll go adjust the registers to the closest valid
+ * value.
+ */
+static void vbe_fixup_regs(VGACommonState *s)
+{
+ uint16_t *r = s->vbe_regs;
+ uint32_t bits, linelength, maxy, offset;
+
+ if (!(r[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED)) {
+ /* vbe is turned off -- nothing to do */
+ return;
+ }
+
+ /* check depth */
+ switch (r[VBE_DISPI_INDEX_BPP]) {
+ case 4:
+ case 8:
+ case 16:
+ case 24:
+ case 32:
+ bits = r[VBE_DISPI_INDEX_BPP];
+ break;
+ case 15:
+ bits = 16;
+ break;
+ default:
+ bits = r[VBE_DISPI_INDEX_BPP] = 8;
+ break;
+ }
+
+ /* check width */
+ r[VBE_DISPI_INDEX_XRES] &= ~7u;
+ if (r[VBE_DISPI_INDEX_XRES] == 0) {
+ r[VBE_DISPI_INDEX_XRES] = 8;
+ }
+ if (r[VBE_DISPI_INDEX_XRES] > VBE_DISPI_MAX_XRES) {
+ r[VBE_DISPI_INDEX_XRES] = VBE_DISPI_MAX_XRES;
+ }
+ r[VBE_DISPI_INDEX_VIRT_WIDTH] &= ~7u;
+ if (r[VBE_DISPI_INDEX_VIRT_WIDTH] > VBE_DISPI_MAX_XRES) {
+ r[VBE_DISPI_INDEX_VIRT_WIDTH] = VBE_DISPI_MAX_XRES;
+ }
+ if (r[VBE_DISPI_INDEX_VIRT_WIDTH] < r[VBE_DISPI_INDEX_XRES]) {
+ r[VBE_DISPI_INDEX_VIRT_WIDTH] = r[VBE_DISPI_INDEX_XRES];
+ }
+
+ /* check height */
+ linelength = r[VBE_DISPI_INDEX_VIRT_WIDTH] * bits / 8;
+ maxy = s->vbe_size / linelength;
+ if (r[VBE_DISPI_INDEX_YRES] == 0) {
+ r[VBE_DISPI_INDEX_YRES] = 1;
+ }
+ if (r[VBE_DISPI_INDEX_YRES] > VBE_DISPI_MAX_YRES) {
+ r[VBE_DISPI_INDEX_YRES] = VBE_DISPI_MAX_YRES;
+ }
+ if (r[VBE_DISPI_INDEX_YRES] > maxy) {
+ r[VBE_DISPI_INDEX_YRES] = maxy;
+ }
+
+ /* check offset */
+ if (r[VBE_DISPI_INDEX_X_OFFSET] > VBE_DISPI_MAX_XRES) {
+ r[VBE_DISPI_INDEX_X_OFFSET] = VBE_DISPI_MAX_XRES;
+ }
+ if (r[VBE_DISPI_INDEX_Y_OFFSET] > VBE_DISPI_MAX_YRES) {
+ r[VBE_DISPI_INDEX_Y_OFFSET] = VBE_DISPI_MAX_YRES;
+ }
+ offset = r[VBE_DISPI_INDEX_X_OFFSET] * bits / 8;
+ offset += r[VBE_DISPI_INDEX_Y_OFFSET] * linelength;
+ if (offset + r[VBE_DISPI_INDEX_YRES] * linelength > s->vbe_size) {
+ r[VBE_DISPI_INDEX_Y_OFFSET] = 0;
+ offset = r[VBE_DISPI_INDEX_X_OFFSET] * bits / 8;
+ if (offset + r[VBE_DISPI_INDEX_YRES] * linelength > s->vbe_size) {
+ r[VBE_DISPI_INDEX_X_OFFSET] = 0;
+ offset = 0;
+ }
+ }
+
+ /* update vga state */
+ r[VBE_DISPI_INDEX_VIRT_HEIGHT] = maxy;
+ s->vbe_line_offset = linelength;
+ s->vbe_start_addr = offset / 4;
+}
+
static uint32_t vbe_ioport_read_index(void *opaque, uint32_t addr)
{
VGACommonState *s = opaque;
@@ -649,22 +736,13 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
}
break;
case VBE_DISPI_INDEX_XRES:
- if ((val <= VBE_DISPI_MAX_XRES) && ((val & 7) == 0)) {
- s->vbe_regs[s->vbe_index] = val;
- }
- break;
case VBE_DISPI_INDEX_YRES:
- if (val <= VBE_DISPI_MAX_YRES) {
- s->vbe_regs[s->vbe_index] = val;
- }
- break;
case VBE_DISPI_INDEX_BPP:
- if (val == 0)
- val = 8;
- if (val == 4 || val == 8 || val == 15 ||
- val == 16 || val == 24 || val == 32) {
- s->vbe_regs[s->vbe_index] = val;
- }
+ case VBE_DISPI_INDEX_VIRT_WIDTH:
+ case VBE_DISPI_INDEX_X_OFFSET:
+ case VBE_DISPI_INDEX_Y_OFFSET:
+ s->vbe_regs[s->vbe_index] = val;
+ vbe_fixup_regs(s);
break;
case VBE_DISPI_INDEX_BANK:
if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4) {
@@ -681,19 +759,11 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
!(s->vbe_regs[VBE_DISPI_INDEX_ENABLE] & VBE_DISPI_ENABLED)) {
int h, shift_control;
- s->vbe_regs[VBE_DISPI_INDEX_VIRT_WIDTH] =
- s->vbe_regs[VBE_DISPI_INDEX_XRES];
- s->vbe_regs[VBE_DISPI_INDEX_VIRT_HEIGHT] =
- s->vbe_regs[VBE_DISPI_INDEX_YRES];
+ s->vbe_regs[VBE_DISPI_INDEX_VIRT_WIDTH] = 0;
s->vbe_regs[VBE_DISPI_INDEX_X_OFFSET] = 0;
s->vbe_regs[VBE_DISPI_INDEX_Y_OFFSET] = 0;
-
- if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4)
- s->vbe_line_offset = s->vbe_regs[VBE_DISPI_INDEX_XRES] >> 1;
- else
- s->vbe_line_offset = s->vbe_regs[VBE_DISPI_INDEX_XRES] *
- ((s->vbe_regs[VBE_DISPI_INDEX_BPP] + 7) >> 3);
- s->vbe_start_addr = 0;
+ s->vbe_regs[VBE_DISPI_INDEX_ENABLE] |= VBE_DISPI_ENABLED;
+ vbe_fixup_regs(s);
/* clear the screen (should be done in BIOS) */
if (!(val & VBE_DISPI_NOCLEARMEM)) {
@@ -742,40 +812,6 @@ void vbe_ioport_write_data(void *opaque, uint32_t addr, uint32_t val)
s->vbe_regs[s->vbe_index] = val;
vga_update_memory_access(s);
break;
- case VBE_DISPI_INDEX_VIRT_WIDTH:
- {
- int w, h, line_offset;
-
- if (val < s->vbe_regs[VBE_DISPI_INDEX_XRES])
- return;
- w = val;
- if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4)
- line_offset = w >> 1;
- else
- line_offset = w * ((s->vbe_regs[VBE_DISPI_INDEX_BPP] + 7) >> 3);
- h = s->vbe_size / line_offset;
- /* XXX: support weird bochs semantics ? */
- if (h < s->vbe_regs[VBE_DISPI_INDEX_YRES])
- return;
- s->vbe_regs[VBE_DISPI_INDEX_VIRT_WIDTH] = w;
- s->vbe_regs[VBE_DISPI_INDEX_VIRT_HEIGHT] = h;
- s->vbe_line_offset = line_offset;
- }
- break;
- case VBE_DISPI_INDEX_X_OFFSET:
- case VBE_DISPI_INDEX_Y_OFFSET:
- {
- int x;
- s->vbe_regs[s->vbe_index] = val;
- s->vbe_start_addr = s->vbe_line_offset * s->vbe_regs[VBE_DISPI_INDEX_Y_OFFSET];
- x = s->vbe_regs[VBE_DISPI_INDEX_X_OFFSET];
- if (s->vbe_regs[VBE_DISPI_INDEX_BPP] == 4)
- s->vbe_start_addr += x >> 1;
- else
- s->vbe_start_addr += x * ((s->vbe_regs[VBE_DISPI_INDEX_BPP] + 7) >> 3);
- s->vbe_start_addr >>= 2;
- }
- break;
default:
break;
}
|
