[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen master] x86/pv: Correct the auditing of guest breakpoint addresses
commit dc9d9aa62ddeb14abd5672690d30789829f58f7e Author: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> AuthorDate: Tue Sep 19 12:13:50 2023 +0100 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Wed Oct 11 06:36:50 2023 +0100 x86/pv: Correct the auditing of guest breakpoint addresses 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@xxxxxxxxxx> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx> Reviewed-by: Jan Beulich <jbeulich@xxxxxxxx> --- xen/arch/x86/domain.c | 2 +- xen/arch/x86/include/asm/debugreg.h | 19 +++++++++++++++++++ xen/arch/x86/pv/misc-hypercalls.c | 2 +- 3 files changed, 21 insertions(+), 2 deletions(-) diff --git a/xen/arch/x86/domain.c b/xen/arch/x86/domain.c index 8d3d52034a..d05ee0da55 100644 --- a/xen/arch/x86/domain.c +++ b/xen/arch/x86/domain.c @@ -1088,7 +1088,7 @@ int arch_set_info_guest( if ( is_pv_domain(d) ) { for ( i = 0; i < ARRAY_SIZE(v->arch.dr); i++ ) - if ( !access_ok(c(debugreg[i]), sizeof(long)) ) + if ( !breakpoint_addr_ok(c(debugreg[i])) ) return -EINVAL; /* * Prior to Xen 4.11, dr5 was used to hold the emulated-only diff --git a/xen/arch/x86/include/asm/debugreg.h b/xen/arch/x86/include/asm/debugreg.h index 39ba312b84..b6454cc04e 100644 --- a/xen/arch/x86/include/asm/debugreg.h +++ b/xen/arch/x86/include/asm/debugreg.h @@ -76,6 +76,25 @@ __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) + struct vcpu; long set_debugreg(struct vcpu *, unsigned int reg, unsigned long value); void activate_debugregs(const struct vcpu *); diff --git a/xen/arch/x86/pv/misc-hypercalls.c b/xen/arch/x86/pv/misc-hypercalls.c index 99f5028128..b529f00ea1 100644 --- a/xen/arch/x86/pv/misc-hypercalls.c +++ b/xen/arch/x86/pv/misc-hypercalls.c @@ -61,7 +61,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; -- generated by git-patchbot for /home/xen/git/xen.git#master
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |