aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorRusty Russell <rusty@rustcorp.com.au>2007-08-09 06:57:13 -0400
committerLinus Torvalds <torvalds@woody.linux-foundation.org>2007-08-09 11:14:56 -0400
commit0d027c01cd36b8cff727c78d2e40d334ba9895a8 (patch)
tree21dde51409ab83cbb3ce7200393b3a0acb76388d
parent37250097e1b730c30da1790e354c0da65e617043 (diff)
lguest: Fix Malicious Guest GDT Host Crash
If a Guest makes hypercall which sets a GDT entry to not present, we currently set any segment registers using that GDT entry to 0. Unfortunately, this is not sufficient: there are other ways of altering GDT entries which will cause a fault. The correct solution to do what Linux does: let them set any GDT value they want and handle the #GP when popping causes a fault. This has the added benefit of making our Switcher slightly more robust in the case of any other bugs which cause it to fault. We kill the Guest if it causes a fault in the Switcher: it's the Guest's responsibility to make sure it's not using segments when it changes them. Signed-off-by: Rusty Russell <rusty@rustcorp.com.au> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r--drivers/lguest/core.c5
-rw-r--r--drivers/lguest/interrupts_and_traps.c9
-rw-r--r--drivers/lguest/lguest.c5
-rw-r--r--drivers/lguest/segments.c62
-rw-r--r--drivers/lguest/switcher.S15
5 files changed, 29 insertions, 67 deletions
diff --git a/drivers/lguest/core.c b/drivers/lguest/core.c
index 0a46e8837d9a..4a315f08a567 100644
--- a/drivers/lguest/core.c
+++ b/drivers/lguest/core.c
@@ -453,6 +453,11 @@ static void run_guest_once(struct lguest *lg, struct lguest_pages *pages)
453 * lguest_pages". */ 453 * lguest_pages". */
454 copy_in_guest_info(lg, pages); 454 copy_in_guest_info(lg, pages);
455 455
456 /* Set the trap number to 256 (impossible value). If we fault while
457 * switching to the Guest (bad segment registers or bug), this will
458 * cause us to abort the Guest. */
459 lg->regs->trapnum = 256;
460
456 /* Now: we push the "eflags" register on the stack, then do an "lcall". 461 /* Now: we push the "eflags" register on the stack, then do an "lcall".
457 * This is how we change from using the kernel code segment to using 462 * This is how we change from using the kernel code segment to using
458 * the dedicated lguest code segment, as well as jumping into the 463 * the dedicated lguest code segment, as well as jumping into the
diff --git a/drivers/lguest/interrupts_and_traps.c b/drivers/lguest/interrupts_and_traps.c
index 49787e964a0d..49aa55577d0d 100644
--- a/drivers/lguest/interrupts_and_traps.c
+++ b/drivers/lguest/interrupts_and_traps.c
@@ -195,13 +195,16 @@ static int has_err(unsigned int trap)
195/* deliver_trap() returns true if it could deliver the trap. */ 195/* deliver_trap() returns true if it could deliver the trap. */
196int deliver_trap(struct lguest *lg, unsigned int num) 196int deliver_trap(struct lguest *lg, unsigned int num)
197{ 197{
198 u32 lo = lg->idt[num].a, hi = lg->idt[num].b; 198 /* Trap numbers are always 8 bit, but we set an impossible trap number
199 * for traps inside the Switcher, so check that here. */
200 if (num >= ARRAY_SIZE(lg->idt))
201 return 0;
199 202
200 /* Early on the Guest hasn't set the IDT entries (or maybe it put a 203 /* Early on the Guest hasn't set the IDT entries (or maybe it put a
201 * bogus one in): if we fail here, the Guest will be killed. */ 204 * bogus one in): if we fail here, the Guest will be killed. */
202 if (!idt_present(lo, hi)) 205 if (!idt_present(lg->idt[num].a, lg->idt[num].b))
203 return 0; 206 return 0;
204 set_guest_interrupt(lg, lo, hi, has_err(num)); 207 set_guest_interrupt(lg, lg->idt[num].a, lg->idt[num].b, has_err(num));
205 return 1; 208 return 1;
206} 209}
207 210
diff --git a/drivers/lguest/lguest.c b/drivers/lguest/lguest.c
index fb17d2757be9..524beea7fb19 100644
--- a/drivers/lguest/lguest.c
+++ b/drivers/lguest/lguest.c
@@ -323,9 +323,12 @@ static void lguest_write_gdt_entry(struct desc_struct *dt,
323 * __thread variables). So we have a hypercall specifically for this case. */ 323 * __thread variables). So we have a hypercall specifically for this case. */
324static void lguest_load_tls(struct thread_struct *t, unsigned int cpu) 324static void lguest_load_tls(struct thread_struct *t, unsigned int cpu)
325{ 325{
326 /* There's one problem which normal hardware doesn't have: the Host
327 * can't handle us removing entries we're currently using. So we clear
328 * the GS register here: if it's needed it'll be reloaded anyway. */
329 loadsegment(gs, 0);
326 lazy_hcall(LHCALL_LOAD_TLS, __pa(&t->tls_array), cpu, 0); 330 lazy_hcall(LHCALL_LOAD_TLS, __pa(&t->tls_array), cpu, 0);
327} 331}
328/*:*/
329 332
330/*G:038 That's enough excitement for now, back to ploughing through each of 333/*G:038 That's enough excitement for now, back to ploughing through each of
331 * the paravirt_ops (we're about 1/3 of the way through). 334 * the paravirt_ops (we're about 1/3 of the way through).
diff --git a/drivers/lguest/segments.c b/drivers/lguest/segments.c
index f675a41a80da..9b81119f46e9 100644
--- a/drivers/lguest/segments.c
+++ b/drivers/lguest/segments.c
@@ -43,22 +43,6 @@
43 * begin. 43 * begin.
44 */ 44 */
45 45
46/* Is the descriptor the Guest wants us to put in OK?
47 *
48 * The flag which Intel says must be zero: must be zero. The descriptor must
49 * be present, (this is actually checked earlier but is here for thorougness),
50 * and the descriptor type must be 1 (a memory segment). */
51static int desc_ok(const struct desc_struct *gdt)
52{
53 return ((gdt->b & 0x00209000) == 0x00009000);
54}
55
56/* Is the segment present? (Otherwise it can't be used by the Guest). */
57static int segment_present(const struct desc_struct *gdt)
58{
59 return gdt->b & 0x8000;
60}
61
62/* There are several entries we don't let the Guest set. The TSS entry is the 46/* There are several entries we don't let the Guest set. The TSS entry is the
63 * "Task State Segment" which controls all kinds of delicate things. The 47 * "Task State Segment" which controls all kinds of delicate things. The
64 * LGUEST_CS and LGUEST_DS entries are reserved for the Switcher, and the 48 * LGUEST_CS and LGUEST_DS entries are reserved for the Switcher, and the
@@ -71,37 +55,11 @@ static int ignored_gdt(unsigned int num)
71 || num == GDT_ENTRY_DOUBLEFAULT_TSS); 55 || num == GDT_ENTRY_DOUBLEFAULT_TSS);
72} 56}
73 57
74/* If the Guest asks us to remove an entry from the GDT, we have to be careful. 58/*H:610 Once the GDT has been changed, we fix the new entries up a little. We
75 * If one of the segment registers is pointing at that entry the Switcher will 59 * don't care if they're invalid: the worst that can happen is a General
76 * crash when it tries to reload the segment registers for the Guest. 60 * Protection Fault in the Switcher when it restores a Guest segment register
77 * 61 * which tries to use that entry. Then we kill the Guest for causing such a
78 * It doesn't make much sense for the Guest to try to remove its own code, data 62 * mess: the message will be "unhandled trap 256". */
79 * or stack segments while they're in use: assume that's a Guest bug. If it's
80 * one of the lesser segment registers using the removed entry, we simply set
81 * that register to 0 (unusable). */
82static void check_segment_use(struct lguest *lg, unsigned int desc)
83{
84 /* GDT entries are 8 bytes long, so we divide to get the index and
85 * ignore the bottom bits. */
86 if (lg->regs->gs / 8 == desc)
87 lg->regs->gs = 0;
88 if (lg->regs->fs / 8 == desc)
89 lg->regs->fs = 0;
90 if (lg->regs->es / 8 == desc)
91 lg->regs->es = 0;
92 if (lg->regs->ds / 8 == desc
93 || lg->regs->cs / 8 == desc
94 || lg->regs->ss / 8 == desc)
95 kill_guest(lg, "Removed live GDT entry %u", desc);
96}
97/*:*/
98/*M:009 We wouldn't need to check for removal of in-use segments if we handled
99 * faults in the Switcher. However, it's probably not a worthwhile
100 * optimization. :*/
101
102/*H:610 Once the GDT has been changed, we look through the changed entries and
103 * see if they're OK. If not, we'll call kill_guest() and the Guest will never
104 * get to use the invalid entries. */
105static void fixup_gdt_table(struct lguest *lg, unsigned start, unsigned end) 63static void fixup_gdt_table(struct lguest *lg, unsigned start, unsigned end)
106{ 64{
107 unsigned int i; 65 unsigned int i;
@@ -112,16 +70,6 @@ static void fixup_gdt_table(struct lguest *lg, unsigned start, unsigned end)
112 if (ignored_gdt(i)) 70 if (ignored_gdt(i))
113 continue; 71 continue;
114 72
115 /* We could fault in switch_to_guest if they are using
116 * a removed segment. */
117 if (!segment_present(&lg->gdt[i])) {
118 check_segment_use(lg, i);
119 continue;
120 }
121
122 if (!desc_ok(&lg->gdt[i]))
123 kill_guest(lg, "Bad GDT descriptor %i", i);
124
125 /* Segment descriptors contain a privilege level: the Guest is 73 /* Segment descriptors contain a privilege level: the Guest is
126 * sometimes careless and leaves this as 0, even though it's 74 * sometimes careless and leaves this as 0, even though it's
127 * running at privilege level 1. If so, we fix it here. */ 75 * running at privilege level 1. If so, we fix it here. */
diff --git a/drivers/lguest/switcher.S b/drivers/lguest/switcher.S
index d418179ea6b5..7c9c230cc845 100644
--- a/drivers/lguest/switcher.S
+++ b/drivers/lguest/switcher.S
@@ -47,6 +47,7 @@
47// Down here in the depths of assembler code. 47// Down here in the depths of assembler code.
48#include <linux/linkage.h> 48#include <linux/linkage.h>
49#include <asm/asm-offsets.h> 49#include <asm/asm-offsets.h>
50#include <asm/page.h>
50#include "lg.h" 51#include "lg.h"
51 52
52// We mark the start of the code to copy 53// We mark the start of the code to copy
@@ -182,13 +183,15 @@ ENTRY(switch_to_guest)
182 movl $(LGUEST_DS), %eax; \ 183 movl $(LGUEST_DS), %eax; \
183 movl %eax, %ds; \ 184 movl %eax, %ds; \
184 /* So where are we? Which CPU, which struct? \ 185 /* So where are we? Which CPU, which struct? \
185 * The stack is our clue: our TSS sets \ 186 * The stack is our clue: our TSS starts \
186 * It at the end of "struct lguest_pages" \ 187 * It at the end of "struct lguest_pages". \
187 * And we then pushed and pushed and pushed Guest regs: \ 188 * Or we may have stumbled while restoring \
188 * Now stack points atop the "struct lguest_regs". \ 189 * Our Guest segment regs while in switch_to_guest, \
189 * Subtract that offset, and we find our struct. */ \ 190 * The fault pushed atop that part-unwound stack. \
191 * If we round the stack down to the page start \
192 * We're at the start of "struct lguest_pages". */ \
190 movl %esp, %eax; \ 193 movl %esp, %eax; \
191 subl $LGUEST_PAGES_regs, %eax; \ 194 andl $(~(1 << PAGE_SHIFT - 1)), %eax; \
192 /* Save our trap number: the switch will obscure it \ 195 /* Save our trap number: the switch will obscure it \
193 * (The Guest regs are not mapped here in the Host) \ 196 * (The Guest regs are not mapped here in the Host) \
194 * %ebx holds it safe for deliver_to_host */ \ 197 * %ebx holds it safe for deliver_to_host */ \