Xen Security Advisory 446 v2 (CVE-2023-46836) - x86: BTC/SRSO fixes not fully effective

Xen.org security team posted 1 patch 5 months, 2 weeks ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/E1r2tyQ-0002ID-NV@xenbits.xenproject.org
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(-)
Xen Security Advisory 446 v2 (CVE-2023-46836) - x86: BTC/SRSO fixes not fully effective
Posted by Xen.org security team 5 months, 2 weeks ago
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2023-46836 / XSA-446
                               version 2

                x86: BTC/SRSO fixes not fully effective

UPDATES IN VERSION 2
====================

Grammar fixes.

Public release.

ISSUE DESCRIPTION
=================

The fixes for XSA-422 (Branch Type Confusion) and XSA-434 (Speculative
Return Stack Overflow) are not IRQ-safe.  It was believed that the
mitigations always operated in contexts with IRQs disabled.

However, the original XSA-254 fix for Meltdown (XPTI) deliberately left
interrupts enabled on two entry paths; one unconditionally, and one
conditionally on whether XPTI was active.

As BTC/SRSO and Meltdown affect different CPU vendors, the mitigations
are not active together by default.  Therefore, there is a race
condition whereby a malicious PV guest can bypass BTC/SRSO protections
and launch a BTC/SRSO attack against Xen.

IMPACT
======

An attacker in a PV guest might be able to infer the contents of memory
belonging to other guests.

VULNERABLE SYSTEMS
==================

All versions of Xen are vulnerable.

Xen is only vulnerable in default configurations on AMD and Hygon CPUs.

Xen is not believed to be vulnerable in default configurations on CPUs
from other hardware vendors.

Only PV guests can leverage the vulnerability.

MITIGATION
==========

Running only HVM or PVH VMs will avoid the vulnerability.

CREDITS
=======

This issue was discovered by Andrew Cooper of XenServer.

RESOLUTION
==========

Applying the appropriate attached patch resolves this issue.

Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball.  Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.

xsa446.patch           xen-unstable - Xen 4.15.x

$ sha256sum xsa446*
ed27ad5f36af31233e25c80daefb8b0078eeb18cacbc1923fdd6f10f0b394201  xsa446.patch
$

DEPLOYMENT DURING EMBARGO
=========================

Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.

But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).

Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.

(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable.  This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)

For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
  http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----

iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmVTfRgMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZfLoH/iZJzkNK4d6vUrx8F5Srm8mAIDMGL4fPvJz00IsO
7h7+/wz0+FdnaWgT/12kHjIJv7p38rNkyJ3UC3p55NFFGUXKQxaKjJ6YU70IdHmY
zbQDdYd2eB9dGbAq2NEkZibtg5mhhThBsQw9Sf+YZuSzOV5xRWiEhnBGz7l4+Dym
bM7vuusZo3/iUc0WgE+p+j85QmzgTFdt7VEUYY2mSTFud+hDYtvx62Ej3AkwCRdu
I0JbGYcRaDR9RPDae2d9yvz0+E473rFgOSX6DqZLjnQ+UQivZ7eo8soJD87qY4Jh
OrEDMQWysSNiT90NYWZ+HxsRRZVjPVPoxX6EWEkwC7+CffI=
=2Xtx
-----END PGP SIGNATURE-----
From 80d5aada598c3a800a350003d5d582931545e13c Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Thu, 26 Oct 2023 14:37:38 +0100
Subject: [PATCH] x86/spec-ctrl: Remove conditional IRQs-on-ness for INT
 $0x80/0x82 paths
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

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

base-commit: 7befef87cc9b1bb8ca15d866ce1ecd9165ccb58c
prerequisite-patch-id: 142a87c707411d49e136c3fb76f1b14963ec6dc8
-- 
2.30.2