summaryrefslogtreecommitdiff
blob: 353a5e3a2ea3b095f98946b9d895bf437c50338b (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
From 29efce0f8f10e381417a61f2f9988b40d4f6bcf0 Mon Sep 17 00:00:00 2001
From: Andrew Cooper <andrew.cooper3@citrix.com>
Date: Tue, 26 Sep 2023 20:06:57 +0100
Subject: [PATCH 27/27] x86/pv: Correct the auditing of guest breakpoint
 addresses
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The use of access_ok() is buggy, because it permits access to the compat
translation area.  64bit PV guests don't use the XLAT area, but on AMD
hardware, the DBEXT feature allows a breakpoint to match up to a 4G aligned
region, allowing the breakpoint to reach outside of the XLAT area.

Prior to c/s cda16c1bb223 ("x86: mirror compat argument translation area for
32-bit PV"), the live GDT was within 4G of the XLAT area.

All together, this allowed a malicious 64bit PV guest on AMD hardware to place
a breakpoint over the live GDT, and trigger a #DB livelock (CVE-2015-8104).

Introduce breakpoint_addr_ok() and explain why __addr_ok() happens to be an
appropriate check in this case.

For Xen 4.14 and later, this is a latent bug because the XLAT area has moved
to be on its own with nothing interesting adjacent.  For Xen 4.13 and older on
AMD hardware, this fixes a PV-trigger-able DoS.

This is part of XSA-444 / CVE-2023-34328.

Fixes: 65e355490817 ("x86/PV: support data breakpoint extension registers")
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
Reviewed-by: Jan Beulich <jbeulich@suse.com>
(cherry picked from commit dc9d9aa62ddeb14abd5672690d30789829f58f7e)
---
 xen/arch/x86/pv/misc-hypercalls.c |  2 +-
 xen/include/asm-x86/debugreg.h    | 20 ++++++++++++++++++++
 2 files changed, 21 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/pv/misc-hypercalls.c b/xen/arch/x86/pv/misc-hypercalls.c
index 5dade24726..681c16108f 100644
--- a/xen/arch/x86/pv/misc-hypercalls.c
+++ b/xen/arch/x86/pv/misc-hypercalls.c
@@ -68,7 +68,7 @@ long set_debugreg(struct vcpu *v, unsigned int reg, unsigned long value)
     switch ( reg )
     {
     case 0 ... 3:
-        if ( !access_ok(value, sizeof(long)) )
+        if ( !breakpoint_addr_ok(value) )
             return -EPERM;
 
         v->arch.dr[reg] = value;
diff --git a/xen/include/asm-x86/debugreg.h b/xen/include/asm-x86/debugreg.h
index c57914efc6..cc29826524 100644
--- a/xen/include/asm-x86/debugreg.h
+++ b/xen/include/asm-x86/debugreg.h
@@ -77,6 +77,26 @@
     asm volatile ( "mov %%db" #reg ",%0" : "=r" (__val) );  \
     __val;                                                  \
 })
+
+/*
+ * Architecturally, %dr{0..3} can have any arbitrary value.  However, Xen
+ * can't allow the guest to breakpoint the Xen address range, so we limit the
+ * guest to the lower canonical half, or above the Xen range in the higher
+ * canonical half.
+ *
+ * Breakpoint lengths are specified to mask the low order address bits,
+ * meaning all breakpoints are naturally aligned.  With %dr7, the widest
+ * breakpoint is 8 bytes.  With DBEXT, the widest breakpoint is 4G.  Both of
+ * the Xen boundaries have >4G alignment.
+ *
+ * In principle we should account for HYPERVISOR_COMPAT_VIRT_START(d), but
+ * 64bit Xen has never enforced this for compat guests, and there's no problem
+ * (to Xen) if the guest breakpoints it's alias of the M2P.  Skipping this
+ * aspect simplifies the logic, and causes us not to reject a migrating guest
+ * which operated fine on prior versions of Xen.
+ */
+#define breakpoint_addr_ok(a) __addr_ok(a)
+
 long set_debugreg(struct vcpu *, unsigned int reg, unsigned long value);
 void activate_debugregs(const struct vcpu *);
 
-- 
2.42.0