Re: [Xen-devel] [PATCH] x86: make certain memory sub-ops return valid values

[Tim Deegan <tim@xxxxxxx>]

At 10:52 +0000 on 27 Feb (1361962378), Jan Beulich wrote:
> When a domain's shared info field "max_pfn" is zero,
> domain_get_maximum_gpfn() so far returned ULONG_MAX, which
> do_memory_op() in turn converted to -1 (i.e. -EPERM). Make the former
> always return a sensible number (i.e. zero if the field was zero) and
> have the latter no longer truncate return values.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Tim Deegan <tim@xxxxxxx> 
(To the extent that this is an interface to x86/mm/ things).

> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -433,7 +433,7 @@ unsigned long domain_get_maximum_gpfn(st
>      if ( is_hvm_domain(d) )
>          return p2m_get_hostp2m(d)->max_mapped_pfn;
>      /* NB. PV guests specify nr_pfns rather than max_pfn so we adjust here. 
> */
> -    return arch_get_max_pfn(d) - 1;
> +    return (arch_get_max_pfn(d) ?: 1) - 1;
>  }
>  
>  void share_xen_page_with_guest(
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -15,7 +15,8 @@ CHECK_TYPE(domid);
>  
>  int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
>  {
> -    int rc, split, op = cmd & MEMOP_CMD_MASK;
> +    int split, op = cmd & MEMOP_CMD_MASK;
> +    long rc;
>      unsigned int start_extent = cmd >> MEMOP_EXTENT_SHIFT;
>  
>      do
> @@ -204,7 +205,7 @@ int compat_memory_op(unsigned int cmd, X
>  
>          rc = do_memory_op(cmd, nat.hnd);
>          if ( rc < 0 )
> -            return rc;
> +            break;
>  
>          cmd = 0;
>          if ( hypercall_xlat_continuation(&cmd, 0x02, nat.hnd, compat) )
> @@ -326,5 +327,11 @@ int compat_memory_op(unsigned int cmd, X
>                  __HYPERVISOR_memory_op, "ih", cmd, compat);
>      } while ( split > 0 );
>  
> +    if ( unlikely(rc > INT_MAX) )
> +        return INT_MAX;
> +
> +    if ( unlikely(rc < INT_MIN) )
> +        return INT_MIN;
> +
>      return rc;
>  }
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -545,14 +545,13 @@ static long memory_exchange(XEN_GUEST_HA
>  long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
>      struct domain *d;
> -    int rc, op;
> +    long rc;
>      unsigned int address_bits;
>      unsigned long start_extent;
>      struct xen_memory_reservation reservation;
>      struct memop_args args;
>      domid_t domid;
> -
> -    op = cmd & MEMOP_CMD_MASK;
> +    int op = cmd & MEMOP_CMD_MASK;
>  
>      switch ( op )
>      {
> 
> 
> 

> x86: make certain memory sub-ops return valid values
> 
> When a domain's shared info field "max_pfn" is zero,
> domain_get_maximum_gpfn() so far returned ULONG_MAX, which
> do_memory_op() in turn converted to -1 (i.e. -EPERM). Make the former
> always return a sensible number (i.e. zero if the field was zero) and
> have the latter no longer truncate return values.
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> 
> --- a/xen/arch/x86/mm.c
> +++ b/xen/arch/x86/mm.c
> @@ -433,7 +433,7 @@ unsigned long domain_get_maximum_gpfn(st
>      if ( is_hvm_domain(d) )
>          return p2m_get_hostp2m(d)->max_mapped_pfn;
>      /* NB. PV guests specify nr_pfns rather than max_pfn so we adjust here. 
> */
> -    return arch_get_max_pfn(d) - 1;
> +    return (arch_get_max_pfn(d) ?: 1) - 1;
>  }
>  
>  void share_xen_page_with_guest(
> --- a/xen/common/compat/memory.c
> +++ b/xen/common/compat/memory.c
> @@ -15,7 +15,8 @@ CHECK_TYPE(domid);
>  
>  int compat_memory_op(unsigned int cmd, XEN_GUEST_HANDLE_PARAM(void) compat)
>  {
> -    int rc, split, op = cmd & MEMOP_CMD_MASK;
> +    int split, op = cmd & MEMOP_CMD_MASK;
> +    long rc;
>      unsigned int start_extent = cmd >> MEMOP_EXTENT_SHIFT;
>  
>      do
> @@ -204,7 +205,7 @@ int compat_memory_op(unsigned int cmd, X
>  
>          rc = do_memory_op(cmd, nat.hnd);
>          if ( rc < 0 )
> -            return rc;
> +            break;
>  
>          cmd = 0;
>          if ( hypercall_xlat_continuation(&cmd, 0x02, nat.hnd, compat) )
> @@ -326,5 +327,11 @@ int compat_memory_op(unsigned int cmd, X
>                  __HYPERVISOR_memory_op, "ih", cmd, compat);
>      } while ( split > 0 );
>  
> +    if ( unlikely(rc > INT_MAX) )
> +        return INT_MAX;
> +
> +    if ( unlikely(rc < INT_MIN) )
> +        return INT_MIN;
> +
>      return rc;
>  }
> --- a/xen/common/memory.c
> +++ b/xen/common/memory.c
> @@ -545,14 +545,13 @@ static long memory_exchange(XEN_GUEST_HA
>  long do_memory_op(unsigned long cmd, XEN_GUEST_HANDLE_PARAM(void) arg)
>  {
>      struct domain *d;
> -    int rc, op;
> +    long rc;
>      unsigned int address_bits;
>      unsigned long start_extent;
>      struct xen_memory_reservation reservation;
>      struct memop_args args;
>      domid_t domid;
> -
> -    op = cmd & MEMOP_CMD_MASK;
> +    int op = cmd & MEMOP_CMD_MASK;
>  
>      switch ( op )
>      {

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


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