Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit a489f0b5 authored by Rusty Russell's avatar Rusty Russell
Browse files

lguest: fix guest crash on non-linear addresses in gdt pvops 



Fixes guest crash 'lguest: bad read address 0x4800000 len 256'

The new per-cpu allocator ends up handing a non-linear address to
write_gdt_entry.  We do __pa() on it, and hand it to the host, which
kills us.

I've long wanted to make the hypercall "LOAD_GDT_ENTRY" to match the IDT
code, but had no pressing reason until now.

Signed-off-by: default avatarRusty Russell <rusty@rustcorp.com.au>
Cc: lguest@ozlabs.org
parent 88df781a

arch/x86/include/asm/lguest_hcall.h

+1 −1
Original line number Original line Diff line number Diff line
@@ -5,7 +5,6 @@ 
#define LHCALL_FLUSH_ASYNC	0

#define LHCALL_FLUSH_ASYNC	0

#define LHCALL_LGUEST_INIT	1

#define LHCALL_LGUEST_INIT	1

#define LHCALL_SHUTDOWN		2

#define LHCALL_SHUTDOWN		2

#define LHCALL_LOAD_GDT		3

#define LHCALL_NEW_PGTABLE	4

#define LHCALL_NEW_PGTABLE	4

#define LHCALL_FLUSH_TLB	5

#define LHCALL_FLUSH_TLB	5

#define LHCALL_LOAD_IDT_ENTRY	6

#define LHCALL_LOAD_IDT_ENTRY	6
@@ -17,6 +16,7 @@ 
#define LHCALL_SET_PMD		15

#define LHCALL_SET_PMD		15

#define LHCALL_LOAD_TLS		16

#define LHCALL_LOAD_TLS		16

#define LHCALL_NOTIFY		17

#define LHCALL_NOTIFY		17

#define LHCALL_LOAD_GDT_ENTRY	18





#define LGUEST_TRAP_ENTRY 0x1F

#define LGUEST_TRAP_ENTRY 0x1F

arch/x86/lguest/boot.c

+9 −7
Original line number Original line Diff line number Diff line
@@ -273,15 +273,15 @@ static void lguest_load_idt(const struct desc_ptr *desc) 
 * controls the entire thing and the Guest asks it to make changes using the

 * controls the entire thing and the Guest asks it to make changes using the

 * LOAD_GDT hypercall.

 * LOAD_GDT hypercall.

 *

 *

 * This is the opposite of the IDT code where we have a LOAD_IDT_ENTRY

 * This is the exactly like the IDT code.

 * hypercall and use that repeatedly to load a new IDT.  I don't think it

 * really matters, but wouldn't it be nice if they were the same?  Wouldn't

 * it be even better if you were the one to send the patch to fix it?

 */

 */

static void lguest_load_gdt(const struct desc_ptr *desc)

static void lguest_load_gdt(const struct desc_ptr *desc)

{

{

	BUG_ON((desc->size + 1) / 8 != GDT_ENTRIES);

	unsigned int i;

	kvm_hypercall2(LHCALL_LOAD_GDT, __pa(desc->address), GDT_ENTRIES);

	struct desc_struct *gdt = (void *)desc->address;



	for (i = 0; i < (desc->size+1)/8; i++)

		kvm_hypercall3(LHCALL_LOAD_GDT_ENTRY, i, gdt[i].a, gdt[i].b);

}

}





/* For a single GDT entry which changes, we do the lazy thing: alter our GDT,

/* For a single GDT entry which changes, we do the lazy thing: alter our GDT,
@@ -291,7 +291,9 @@ static void lguest_write_gdt_entry(struct desc_struct *dt, int entrynum, 
				   const void *desc, int type)

				   const void *desc, int type)

{

{

	native_write_gdt_entry(dt, entrynum, desc, type);

	native_write_gdt_entry(dt, entrynum, desc, type);

	kvm_hypercall2(LHCALL_LOAD_GDT, __pa(dt), GDT_ENTRIES);

	/* Tell Host about this new entry. */

	kvm_hypercall3(LHCALL_LOAD_GDT_ENTRY, entrynum,

		       dt[entrynum].a, dt[entrynum].b);

}

}





/* OK, I lied.  There are three "thread local storage" GDT entries which change

/* OK, I lied.  There are three "thread local storage" GDT entries which change

drivers/lguest/lg.h

+2 −1
Original line number Original line Diff line number Diff line
@@ -158,7 +158,8 @@ void free_interrupts(void); 
/* segments.c: */

/* segments.c: */

void setup_default_gdt_entries(struct lguest_ro_state *state);

void setup_default_gdt_entries(struct lguest_ro_state *state);

void setup_guest_gdt(struct lg_cpu *cpu);

void setup_guest_gdt(struct lg_cpu *cpu);

void load_guest_gdt(struct lg_cpu *cpu, unsigned long table, u32 num);

void load_guest_gdt_entry(struct lg_cpu *cpu, unsigned int i,

			  u32 low, u32 hi);

void guest_load_tls(struct lg_cpu *cpu, unsigned long tls_array);

void guest_load_tls(struct lg_cpu *cpu, unsigned long tls_array);

void copy_gdt(const struct lg_cpu *cpu, struct desc_struct *gdt);

void copy_gdt(const struct lg_cpu *cpu, struct desc_struct *gdt);

void copy_gdt_tls(const struct lg_cpu *cpu, struct desc_struct *gdt);

void copy_gdt_tls(const struct lg_cpu *cpu, struct desc_struct *gdt);

drivers/lguest/segments.c

+7 −6
Original line number Original line Diff line number Diff line
@@ -144,18 +144,19 @@ void copy_gdt(const struct lg_cpu *cpu, struct desc_struct *gdt) 
			gdt[i] = cpu->arch.gdt[i];

			gdt[i] = cpu->arch.gdt[i];

}

}





/*H:620 This is where the Guest asks us to load a new GDT (LHCALL_LOAD_GDT).

/*H:620 This is where the Guest asks us to load a new GDT entry

 * We copy it from the Guest and tweak the entries. */

 * (LHCALL_LOAD_GDT_ENTRY).  We tweak the entry and copy it in. */

void load_guest_gdt(struct lg_cpu *cpu, unsigned long table, u32 num)

void load_guest_gdt_entry(struct lg_cpu *cpu, u32 num, u32 lo, u32 hi)

{

{

	/* We assume the Guest has the same number of GDT entries as the

	/* We assume the Guest has the same number of GDT entries as the

	 * Host, otherwise we'd have to dynamically allocate the Guest GDT. */

	 * Host, otherwise we'd have to dynamically allocate the Guest GDT. */

	if (num > ARRAY_SIZE(cpu->arch.gdt))

	if (num > ARRAY_SIZE(cpu->arch.gdt))

		kill_guest(cpu, "too many gdt entries %i", num);

		kill_guest(cpu, "too many gdt entries %i", num);





	/* We read the whole thing in, then fix it up. */

	/* Set it up, then fix it. */

	__lgread(cpu, cpu->arch.gdt, table, num * sizeof(cpu->arch.gdt[0]));

	cpu->arch.gdt[num].a = lo;

	fixup_gdt_table(cpu, 0, ARRAY_SIZE(cpu->arch.gdt));

	cpu->arch.gdt[num].b = hi;

	fixup_gdt_table(cpu, num, num+1);

	/* Mark that the GDT changed so the core knows it has to copy it again,

	/* Mark that the GDT changed so the core knows it has to copy it again,

	 * even if the Guest is run on the same CPU. */

	 * even if the Guest is run on the same CPU. */

	cpu->changed |= CHANGED_GDT;

	cpu->changed |= CHANGED_GDT;

drivers/lguest/x86/core.c

+2 −2
Original line number Original line Diff line number Diff line
@@ -568,8 +568,8 @@ void __exit lguest_arch_host_fini(void) 
int lguest_arch_do_hcall(struct lg_cpu *cpu, struct hcall_args *args)

int lguest_arch_do_hcall(struct lg_cpu *cpu, struct hcall_args *args)

{

{

	switch (args->arg0) {

	switch (args->arg0) {

	case LHCALL_LOAD_GDT:

	case LHCALL_LOAD_GDT_ENTRY:

		load_guest_gdt(cpu, args->arg1, args->arg2);

		load_guest_gdt_entry(cpu, args->arg1, args->arg2, args->arg3);

		break;

		break;

	case LHCALL_LOAD_IDT_ENTRY:

	case LHCALL_LOAD_IDT_ENTRY:

		load_guest_idt_entry(cpu, args->arg1, args->arg2, args->arg3);

		load_guest_idt_entry(cpu, args->arg1, args->arg2, args->arg3);