| From 0594c58161b6e0f3da8efa9c6e3d4ba52b652717 Mon Sep 17 00:00:00 2001 |
| From: Jan Beulich <jbeulich@suse.com> |
| Date: Mon, 20 Sep 2021 18:15:11 +0200 |
| Subject: xen/x86: fix PV trap handling on secondary processors |
| |
| From: Jan Beulich <jbeulich@suse.com> |
| |
| commit 0594c58161b6e0f3da8efa9c6e3d4ba52b652717 upstream. |
| |
| The initial observation was that in PV mode under Xen 32-bit user space |
| didn't work anymore. Attempts of system calls ended in #GP(0x402). All |
| of the sudden the vector 0x80 handler was not in place anymore. As it |
| turns out up to 5.13 redundant initialization did occur: Once from |
| cpu_initialize_context() (through its VCPUOP_initialise hypercall) and a |
| 2nd time while each CPU was brought fully up. This 2nd initialization is |
| now gone, uncovering that the 1st one was flawed: Unlike for the |
| set_trap_table hypercall, a full virtual IDT needs to be specified here; |
| the "vector" fields of the individual entries are of no interest. With |
| many (kernel) IDT entries still(?) (i.e. at that point at least) empty, |
| the syscall vector 0x80 ended up in slot 0x20 of the virtual IDT, thus |
| becoming the domain's handler for vector 0x20. |
| |
| Make xen_convert_trap_info() fit for either purpose, leveraging the fact |
| that on the xen_copy_trap_info() path the table starts out zero-filled. |
| This includes moving out the writing of the sentinel, which would also |
| have lead to a buffer overrun in the xen_copy_trap_info() case if all |
| (kernel) IDT entries were populated. Convert the writing of the sentinel |
| to clearing of the entire table entry rather than just the address |
| field. |
| |
| (I didn't bother trying to identify the commit which uncovered the issue |
| in 5.14; the commit named below is the one which actually introduced the |
| bad code.) |
| |
| Fixes: f87e4cac4f4e ("xen: SMP guest support") |
| Cc: stable@vger.kernel.org |
| Signed-off-by: Jan Beulich <jbeulich@suse.com> |
| Reviewed-by: Boris Ostrovsky <boris.ostrovsky@oracle.com> |
| Link: https://lore.kernel.org/r/7a266932-092e-b68f-f2bb-1473b61adc6e@suse.com |
| Signed-off-by: Juergen Gross <jgross@suse.com> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| --- |
| arch/x86/xen/enlighten.c | 15 +++++++++------ |
| 1 file changed, 9 insertions(+), 6 deletions(-) |
| |
| --- a/arch/x86/xen/enlighten.c |
| +++ b/arch/x86/xen/enlighten.c |
| @@ -861,8 +861,8 @@ static void xen_write_idt_entry(gate_des |
| preempt_enable(); |
| } |
| |
| -static void xen_convert_trap_info(const struct desc_ptr *desc, |
| - struct trap_info *traps) |
| +static unsigned xen_convert_trap_info(const struct desc_ptr *desc, |
| + struct trap_info *traps, bool full) |
| { |
| unsigned in, out, count; |
| |
| @@ -872,17 +872,18 @@ static void xen_convert_trap_info(const |
| for (in = out = 0; in < count; in++) { |
| gate_desc *entry = (gate_desc*)(desc->address) + in; |
| |
| - if (cvt_gate_to_trap(in, entry, &traps[out])) |
| + if (cvt_gate_to_trap(in, entry, &traps[out]) || full) |
| out++; |
| } |
| - traps[out].address = 0; |
| + |
| + return out; |
| } |
| |
| void xen_copy_trap_info(struct trap_info *traps) |
| { |
| const struct desc_ptr *desc = this_cpu_ptr(&idt_desc); |
| |
| - xen_convert_trap_info(desc, traps); |
| + xen_convert_trap_info(desc, traps, true); |
| } |
| |
| /* Load a new IDT into Xen. In principle this can be per-CPU, so we |
| @@ -892,6 +893,7 @@ static void xen_load_idt(const struct de |
| { |
| static DEFINE_SPINLOCK(lock); |
| static struct trap_info traps[257]; |
| + unsigned out; |
| |
| trace_xen_cpu_load_idt(desc); |
| |
| @@ -899,7 +901,8 @@ static void xen_load_idt(const struct de |
| |
| memcpy(this_cpu_ptr(&idt_desc), desc, sizeof(idt_desc)); |
| |
| - xen_convert_trap_info(desc, traps); |
| + out = xen_convert_trap_info(desc, traps, false); |
| + memset(&traps[out], 0, sizeof(traps[0])); |
| |
| xen_mc_flush(); |
| if (HYPERVISOR_set_trap_table(traps)) |