All of lore.kernel.org
 help / color / mirror / Atom feed
From: Krystian Hebel <krystian.hebel@3mdeb.com>
To: xen-devel@lists.xenproject.org
Cc: "Andrew Cooper" <andrew.cooper3@citrix.com>,
	"Jan Beulich" <jbeulich@suse.com>,
	"Roger Pau Monné" <roger.pau@citrix.com>, "Wei Liu" <wl@xen.org>
Subject: [PATCH 01/10] x86/spec-ctrl: Remove conditional IRQs-on-ness for INT $0x80/0x82 paths
Date: Tue, 14 Nov 2023 18:49:57 +0100	[thread overview]
Message-ID: <a48bb129f1b9ff55c22cf6d2b589247c8ba3b10e.1699981248.git.krystian.hebel@3mdeb.com> (raw)
In-Reply-To: <cover.1699981248.git.krystian.hebel@3mdeb.com>

From: Andrew Cooper <andrew.cooper3@citrix.com>

Before speculation defences, some paths in Xen could genuinely get away with
being IRQs-on at entry.  But XPTI invalidated this property on most paths, and
attempting to maintain it on the remaining paths was a mistake.

Fast forward, and DO_SPEC_CTRL_COND_IBPB (protection for AMD BTC/SRSO) is not
IRQ-safe, running with IRQs enabled in some cases.  The other actions taken on
these paths happen to be IRQ-safe.

Make entry_int82() and int80_direct_trap() unconditionally Interrupt Gates
rather than Trap Gates.  Remove the conditional re-adjustment of
int80_direct_trap() in smp_prepare_cpus(), and have entry_int82() explicitly
enable interrupts when safe to do so.

In smp_prepare_cpus(), with the conditional re-adjustment removed, the
clearing of pv_cr3 is the only remaining action gated on XPTI, and it is out
of place anyway, repeating work already done by smp_prepare_boot_cpu().  Drop
the entire if() condition to avoid leaving an incorrect vestigial remnant.

Also drop comments which make incorrect statements about when its safe to
enable interrupts.

This is XSA-446 / CVE-2023-46836

Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
---
 xen/arch/x86/pv/traps.c            |  4 ++--
 xen/arch/x86/smpboot.c             | 14 --------------
 xen/arch/x86/x86_64/compat/entry.S |  2 ++
 xen/arch/x86/x86_64/entry.S        |  1 -
 4 files changed, 4 insertions(+), 17 deletions(-)

diff --git a/xen/arch/x86/pv/traps.c b/xen/arch/x86/pv/traps.c
index 74f333da7e1c..240d1a2db7a3 100644
--- a/xen/arch/x86/pv/traps.c
+++ b/xen/arch/x86/pv/traps.c
@@ -139,11 +139,11 @@ void __init pv_trap_init(void)
 #ifdef CONFIG_PV32
     /* The 32-on-64 hypercall vector is only accessible from ring 1. */
     _set_gate(idt_table + HYPERCALL_VECTOR,
-              SYS_DESC_trap_gate, 1, entry_int82);
+              SYS_DESC_irq_gate, 1, entry_int82);
 #endif
 
     /* Fast trap for int80 (faster than taking the #GP-fixup path). */
-    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_trap_gate, 3,
+    _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_irq_gate, 3,
               &int80_direct_trap);
 
     open_softirq(NMI_SOFTIRQ, nmi_softirq);
diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index 3a1a659082c6..4c54ecbc91d7 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1158,20 +1158,6 @@ void __init smp_prepare_cpus(void)
 
     stack_base[0] = (void *)((unsigned long)stack_start & ~(STACK_SIZE - 1));
 
-    if ( opt_xpti_hwdom || opt_xpti_domu )
-    {
-        get_cpu_info()->pv_cr3 = 0;
-
-#ifdef CONFIG_PV
-        /*
-         * All entry points which may need to switch page tables have to start
-         * with interrupts off. Re-write what pv_trap_init() has put there.
-         */
-        _set_gate(idt_table + LEGACY_SYSCALL_VECTOR, SYS_DESC_irq_gate, 3,
-                  &int80_direct_trap);
-#endif
-    }
-
     set_nr_sockets();
 
     socket_cpumask = xzalloc_array(cpumask_t *, nr_sockets);
diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index bd5abd8040bd..fcc3a721f147 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -21,6 +21,8 @@ ENTRY(entry_int82)
         SPEC_CTRL_ENTRY_FROM_PV /* Req: %rsp=regs/cpuinfo, %rdx=0, Clob: acd */
         /* WARNING! `ret`, `call *`, `jmp *` not safe before this point. */
 
+        sti
+
         CR4_PV32_RESTORE
 
         GET_CURRENT(bx)
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 5ca74f5f62b2..9a7b129aa7e4 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -327,7 +327,6 @@ ENTRY(sysenter_entry)
 #ifdef CONFIG_XEN_SHSTK
         ALTERNATIVE "", "setssbsy", X86_FEATURE_XEN_SHSTK
 #endif
-        /* sti could live here when we don't switch page tables below. */
         pushq $FLAT_USER_SS
         pushq $0
         pushfq
-- 
2.41.0



       reply	other threads:[~2023-11-14 17:57 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <cover.1699981248.git.krystian.hebel@3mdeb.com>
2023-11-14 17:49 ` Krystian Hebel [this message]
2023-11-14 18:33   ` [PATCH 01/10] x86/spec-ctrl: Remove conditional IRQs-on-ness for INT $0x80/0x82 paths Krystian Hebel
2023-11-14 17:49 ` [PATCH 02/10] x86/boot: choose AP stack based on APIC ID Krystian Hebel
2023-11-14 17:50 ` [PATCH 03/10] x86: don't access x86_cpu_to_apicid[] directly, use cpu_physical_id(cpu) Krystian Hebel
2023-11-14 17:50 ` [PATCH 04/10] x86/smp: drop x86_cpu_to_apicid, use cpu_data[cpu].apicid instead Krystian Hebel
2023-11-14 17:50 ` [PATCH 05/10] x86/smp: move stack_base to cpu_data Krystian Hebel
2023-11-14 17:50 ` [PATCH 06/10] x86/smp: call x2apic_ap_setup() earlier Krystian Hebel
2023-11-14 17:50 ` [PATCH 07/10] x86/shutdown: protect against recurrent machine_restart() Krystian Hebel
2023-11-14 17:50 ` [PATCH 08/10] x86/smp: drop booting_cpu variable Krystian Hebel
2023-11-14 17:50 ` [PATCH 09/10] x86/smp: make cpu_state per-CPU Krystian Hebel
2023-11-14 17:50 ` [PATCH 10/10] x86/smp: start APs in parallel during boot Krystian Hebel

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a48bb129f1b9ff55c22cf6d2b589247c8ba3b10e.1699981248.git.krystian.hebel@3mdeb.com \
    --to=krystian.hebel@3mdeb.com \
    --cc=andrew.cooper3@citrix.com \
    --cc=jbeulich@suse.com \
    --cc=roger.pau@citrix.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.