Xen Security Advisory 339 v3 (CVE-2020-25596) - x86 pv guest kernel DoS via SYSENTER

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

            Xen Security Advisory CVE-2020-25596 / XSA-339
                               version 3

                 x86 pv guest kernel DoS via SYSENTER

UPDATES IN VERSION 3
====================

Public release.

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

The SYSENTER instruction leaves various state sanitization activities
to software.  One of Xen's sanitization paths injects a #GP fault, and
incorrectly delivers it twice to the guest.

This causes the guest kernel to observe a kernel-privilege #GP fault
(typically fatal) rather than a user-privilege #GP fault (usually
converted into SIGSEGV/etc).

IMPACT
======

Malicious or buggy userspace can crash the guest kernel, resulting in
a VM Denial of Service.

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

All versions of Xen from 3.2 onwards are vulnerable.

Only x86 systems are vulnerable.  ARM platforms are not vulnerable.

Only x86 systems which support the SYSENTER instruction in 64bit mode
are vulnerable.  This is believed to be Intel, Centaur and Shanghai
CPUs.  AMD and Hygon CPUs are not believed to be vulnerable.

Only x86 PV guests can exploit the vulnerability.  x86 PVH / HVM
guests cannot exploit the vulnerability.

MITIGATION
==========

Running only x86 PVH / HVM guests avoids the vulnerability.

CREDITS
=======

This issue was discovered by Andrew Cooper of Citrix.

RESOLUTION
==========

Applying the 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.

xsa339.patch           Xen 4.10 - xen-unstable

$ sha256sum xsa339*
5cece13878cc40b32bc5753c0ef64f989f9b1c7f9549d62ea4fcd06e9620de9e  xsa339.meta
b6ffa7671d905aa12498ad64915be3b7cba74ce1c5bf6bce18b1f106ebf6d715  xsa339.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/4UyVfoK9kFAl9p/ecMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZgEUH/1/5DUgXRKzwvYuERdBintUdCUaezYpjY0VEJ/v5
nPXEZDDkBFZZxtWmLg6gqMsJg4O6npTcZ6Z3ZpP8xTiRexr0fHHRY5FHqOCW0aS+
c0WYQzSvfDW1L/m9fjwsbFKKRCmrwE24L/Jc7GZJlpps22f1mZpn3cwsjidlofHi
WxqpdAPNDLsPDF3+iwt5a8gL3onyeo03MaBhO29UAJIKCo4hxiKu5/e3upXFBdN2
Z4Pyr79E51SiCGxZ/A1NTil9+FyYkP1DgBQdJ6pVrxMnZUhdcjbGLEbrUNaTfgox
yORU8rE3XS2ZajRpW3D2CIGnKJj3zGWaQqx+FufX1m6Y8qE=
=tkQp
-----END PGP SIGNATURE-----

From: Andrew Cooper <[email protected]>
Subject: x86/pv: Avoid double exception injection

There is at least one path (SYSENTER with NT set, Xen converts to #GP) which
ends up injecting the #GP fault twice, first in compat_sysenter(), and then a
second time in compat_test_all_events(), due to the stale TBF_EXCEPTION left
in TRAPBOUNCE_flags.

The guest kernel sees the second fault first, which is a kernel level #GP
pointing at the head of the #GP handler, and is therefore a userspace
trigger-able DoS.

This particular bug has bitten us several times before, so rearrange
{compat_,}create_bounce_frame() to clobber TRAPBOUNCE on success, rather than
leaving this task to one area of code which isn't used uniformly.

Other scenarios which might result in a double injection (e.g. two calls
directly to compat_create_bounce_frame) will now crash the guest, which is far
more obvious than letting the kernel run with corrupt state.

This is XSA-339

Fixes: fdac9515607b ("x86: clear EFLAGS.NT in SYSENTER entry path")
Signed-off-by: Andrew Cooper <[email protected]>
Reviewed-by: Jan Beulich <[email protected]>

diff --git a/xen/arch/x86/x86_64/compat/entry.S b/xen/arch/x86/x86_64/compat/entry.S
index c3e62f8734..73619f57ca 100644
--- a/xen/arch/x86/x86_64/compat/entry.S
+++ b/xen/arch/x86/x86_64/compat/entry.S
@@ -78,7 +78,6 @@ compat_process_softirqs:
         sti
 .Lcompat_bounce_exception:
         call  compat_create_bounce_frame
-        movb  $0, TRAPBOUNCE_flags(%rdx)
         jmp   compat_test_all_events
 
 	ALIGN
@@ -352,7 +351,13 @@ __UNLIKELY_END(compat_bounce_null_selector)
         movl  %eax,UREGS_cs+8(%rsp)
         movl  TRAPBOUNCE_eip(%rdx),%eax
         movl  %eax,UREGS_rip+8(%rsp)
+
+        /* Trapbounce complete.  Clobber state to avoid an erroneous second injection. */
+        xor   %eax, %eax
+        mov   %ax,  TRAPBOUNCE_cs(%rdx)
+        mov   %al,  TRAPBOUNCE_flags(%rdx)
         ret
+
 .section .fixup,"ax"
 .Lfx13:
         xorl  %edi,%edi
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 1e880eb9f6..71a00e846b 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -90,7 +90,6 @@ process_softirqs:
         sti
 .Lbounce_exception:
         call  create_bounce_frame
-        movb  $0, TRAPBOUNCE_flags(%rdx)
         jmp   test_all_events
 
         ALIGN
@@ -512,6 +511,11 @@ UNLIKELY_START(z, create_bounce_frame_bad_bounce_ip)
         jmp   asm_domain_crash_synchronous  /* Does not return */
 __UNLIKELY_END(create_bounce_frame_bad_bounce_ip)
         movq  %rax,UREGS_rip+8(%rsp)
+
+        /* Trapbounce complete.  Clobber state to avoid an erroneous second injection. */
+        xor   %eax, %eax
+        mov   %rax, TRAPBOUNCE_eip(%rdx)
+        mov   %al,  TRAPBOUNCE_flags(%rdx)
         ret
 
         .pushsection .fixup, "ax", @progbits