linux-kernel - [PATCH] xen/x86: fix PV trap handling on secondary processors

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [thread-next>] [day] [month] [year] [list]

Message-ID: <34898e9c-5883-a978-98ee-b81b22d8caed@suse.com>
Date:   Thu, 16 Sep 2021 17:04:02 +0200
From:   Jan Beulich <jbeulich@...e.com>
To:     Juergen Gross <jgross@...e.com>,
        Boris Ostrovsky <boris.ostrovsky@...cle.com>
Cc:     Stefano Stabellini <sstabellini@...nel.org>,
        lkml <linux-kernel@...r.kernel.org>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>
Subject: [PATCH] xen/x86: fix PV trap handling on secondary processors

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.

Since xen_copy_trap_info() has just this single purpose, simply adjust
that function. xen_convert_trap_info() cannot be used here. Its use
would also have lead to a buffer overrun if all (kernel) IDT entries
were populated, due to the function setting a sentinel entry at the end.

(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@...r.kernel.org
Signed-off-by: Jan Beulich <jbeulich@...e.com>
---
In how far it is correct to use the current CPU's IDT is unclear to me.
Looks at least like another latent trap.

--- a/arch/x86/xen/enlighten_pv.c
+++ b/arch/x86/xen/enlighten_pv.c
@@ -775,8 +775,15 @@ static void xen_convert_trap_info(const
 void xen_copy_trap_info(struct trap_info *traps)
 {
 	const struct desc_ptr *desc = this_cpu_ptr(&idt_desc);
+	unsigned i, count = (desc->size + 1) / sizeof(gate_desc);

-	xen_convert_trap_info(desc, traps);
+	BUG_ON(count > 256);
+
+	for (i = 0; i < count; ++i) {
+		const gate_desc *entry = (gate_desc *)desc->address + i;
+
+		cvt_gate_to_trap(i, entry, &traps[i]);
+	}
 }

 /* Load a new IDT into Xen.  In principle this can be per-CPU, so we