[Xen-devel] [PATCH 1/2] x86/HVM: simplify do_hvm_op()

["Jan Beulich" <JBeulich@xxxxxxxx>]

- boundary checks in HVMOP_modified_memory, HVMOP_set_mem_type, and
  HVMOP_set_mem_access: all of these already check for overflow, so
  there's no need to range check the first _and_ last PFN (checking
  the last one suffices)
- copying back interface structures that were previously copied from
  guest memory can use __copy_to_...(), since copy_from_...() already
  did the address range validation

Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

--- a/xen/arch/x86/hvm/hvm.c
+++ b/xen/arch/x86/hvm/hvm.c
@@ -4288,7 +4288,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
                 a.value = d->arch.hvm_domain.params[a.index];
                 break;
             }
-            rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+            rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
         }
 
         HVM_DBG_LOG(DBG_LEVEL_HCALL, "%s param %u = %"PRIx64,
@@ -4386,8 +4386,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             goto param_fail3;
 
         rc = -EINVAL;
-        if ( (a.first_pfn > domain_get_maximum_gpfn(d)) ||
-             ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
+        if ( ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
              ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
             goto param_fail3;
 
@@ -4416,7 +4415,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             /* Check for continuation if it's not the last interation */
             if ( a.nr > 0 && hypercall_preempt_check() )
             {
-                if ( copy_to_guest(arg, &a, 1) )
+                if ( __copy_to_guest(arg, &a, 1) )
                     rc = -EFAULT;
                 else
                     rc = -EAGAIN;
@@ -4465,7 +4464,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
                 a.mem_type =  HVMMEM_ram_rw;
             else
                 a.mem_type =  HVMMEM_mmio_dm;
-            rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+            rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
         }
 
     param_fail_getmemtype:
@@ -4501,8 +4500,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             goto param_fail4;
 
         rc = -EINVAL;
-        if ( (a.first_pfn > domain_get_maximum_gpfn(d)) ||
-             ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
+        if ( ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
              ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d)) )
             goto param_fail4;
             
@@ -4558,7 +4556,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             /* Check for continuation if it's not the last interation */
             if ( a.nr > 0 && hypercall_preempt_check() )
             {
-                if ( copy_to_guest(arg, &a, 1) )
+                if ( __copy_to_guest(arg, &a, 1) )
                     rc = -EFAULT;
                 else
                     rc = -EAGAIN;
@@ -4595,9 +4593,8 @@ long do_hvm_op(unsigned long op, XEN_GUE
 
         rc = -EINVAL;
         if ( (a.first_pfn != ~0ull) &&
-             ((a.first_pfn > domain_get_maximum_gpfn(d)) ||
-             ((a.first_pfn + a.nr - 1) < a.first_pfn) ||
-             ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d))) )
+             (((a.first_pfn + a.nr - 1) < a.first_pfn) ||
+              ((a.first_pfn + a.nr - 1) > domain_get_maximum_gpfn(d))) )
             goto param_fail5;
             
         rc = p2m_set_mem_access(d, a.first_pfn, a.nr, a.hvmmem_access);
@@ -4646,7 +4643,7 @@ long do_hvm_op(unsigned long op, XEN_GUE
             goto param_fail6;
 
         a.hvmmem_access = access;
-        rc = copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
+        rc = __copy_to_guest(arg, &a, 1) ? -EFAULT : 0;
 
     param_fail6:
         rcu_unlock_domain(d);

Attachment: x86-HVM-op-simplify.patch
Description: Text document

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel