[v2] x86/PV: don't use access_ok() in set_debugreg()

Commit Message

Jan Beulich Sept. 19, 2023, 8:44 a.m. UTC

access_ok() is not be applicable here; we really only want a linear
address check for breakpoint addresses, as putting those in debug
register isn't going to result in actual memory accesses. Furthermore
access_ok() assumes to be acting on current, which isn't the case here
when called from arch_set_info_guest().

Note that access_ok() was too lax anyway for 32-bit domains.

Signed-off-by: Jan Beulich <[e-mail]>
---
The questionable use of sizeof(long) is left in place for the moment,
as it's not clear how to best deal with the upper bound of breakpoint
covered ranges: We'd like those to not cover Xen space.
---
v2: Duplicate the change to arch_set_info_guest().

Patch

--- a/xen/arch/x86/domain.c
+++ b/xen/arch/x86/domain.c
@@ -1085,8 +1085,15 @@  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)) )
+        {
+            unsigned long val = c(debugreg[i]);
+
+            if ( is_pv_32bit_domain(d)
+                 ? val + sizeof(long) > HYPERVISOR_COMPAT_VIRT_START(d)
+                 : val + sizeof(long) > (1UL << (VADDR_BITS - 1)) &&
+                   val < HYPERVISOR_VIRT_END )
                 return -EINVAL;
+        }
         /*
          * Prior to Xen 4.11, dr5 was used to hold the emulated-only
          * subset of dr7, and dr4 was unused.
--- a/xen/arch/x86/pv/misc-hypercalls.c
+++ b/xen/arch/x86/pv/misc-hypercalls.c
@@ -61,7 +61,10 @@  long set_debugreg(struct vcpu *v, unsign
     switch ( reg )
     {
     case 0 ... 3:
-        if ( !access_ok(value, sizeof(long)) )
+        if ( is_pv_32bit_vcpu(v)
+             ? value + sizeof(long) > HYPERVISOR_COMPAT_VIRT_START(v->domain)
+             : value + sizeof(long) > (1UL << (VADDR_BITS - 1)) &&
+               value < HYPERVISOR_VIRT_END )
             return -EPERM;
 
         v->arch.dr[reg] = value;