[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Tue, 19 Sep 2023 10:44:10 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-AntiSpam-MessageData-ChunkCount:X-MS-Exchange-AntiSpam-MessageData-0:X-MS-Exchange-AntiSpam-MessageData-1; bh=ohUI3cXFUhetr9WRjFGO0/f3nOBF1PqukznpsDvyA1c=; b=DWqqGyazQqGx1yE04/icRixNOUssqvXAdEOZqQ8crAwxlFScQziHxfWameG7FQ/I1XaX0fkQkuTYReHO7CzSnEvVp5ZqWLEukqPUxdxZzZzXu6Jg/phKh+7SydMkqxwRW/xtN0nWjiGqxXWjpb98Pu4eadALU6g4SSzE+OdatJXfUNr5iNh2qQZFLT7bpytYpoNzVvCIOuqjgzZ1JLB/KOUSCbYbL1hZWY+Ot4qpFvZdypRhb+xGI/RFmIxpYYHpe73VigEn0RmCu/DdhMEjwd2Ame7BsxGxGw7iZ6Dq+4XAHstXvqDJfcWgObVqT2ZxnWh7emCoenrjzuuhXOCs7g==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=M1dJzkuRVja2D90uAp74XUu5LN2TT7SHzCubL9yFUIxkq5M9QSUyz4hSaXnrOLnDgIg44Z4GaKQuSIPd7tnsFl5XZ54slXeRtEdbMMFo/bXeQWjVK/tWAsbkBPn8WOXKLpjXxc20IkLhPD9Lns7fRLqb01WlMST6rXLkGcIp9ZZNUAzuBdwa+RSQhz5KTqtggj3vmHwKYHVm9QgfjMgBsaf5JNR/1U+V7DAomNqQOj+ngJk2LhJ9yVOucdAF+buWTqzAmJfwxSDDp6DubMT64ylBD64yCtzbh5c6UKIlAdP5ZPQEnWCDm41VWRitBUSf0JcWHjmbnZ2x9hFCVyUW3g==
  • Authentication-results: dkim=none (message not signed) header.d=none;dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>, Roger Pau Monné <roger.pau@xxxxxxxxxx>
  • Delivery-date: Tue, 19 Sep 2023 08:44:22 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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 <jbeulich@xxxxxxxx>
---
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().

--- 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;



 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.