On 25/03/14 16:03, Jan Beulich wrote:
> - 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@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- 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);
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel