Re: [Xen-devel] [PATCH v2 08/13] libxc: Check xc_domain_maximum_gpfn for negative return values

[Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>]

> How about this (compile tested but not yet runtime tested):

Runtime tested now too.

I've put the whole lot (including this patch) on 

 git://xenbits.xen.org/people/konradwilk/xen.git xc_cleanup.v3

To ease pulling it in.

Thank you!
> 
> 
> From 92085d29b7e2906095a2bc6849b5a17b478e5c79 Mon Sep 17 00:00:00 2001
> From: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> Date: Fri, 13 Mar 2015 14:57:44 -0400
> Subject: [PATCH v3] libxc: Check xc_domain_maximum_gpfn for negative return
>  values
> 
> Instead of assuming everything is always OK. We stash
> the gpfns value as an parameter. Since we use it in three
> of places we might as well stick it in a common file for
> all three of them to use.
> 
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>
> ---
>  tools/libxc/xc_core_arm.c    | 11 ++++-------
>  tools/libxc/xc_core_x86.c    | 18 ++++++++++--------
>  tools/libxc/xc_domain_save.c |  6 +++++-
>  tools/libxc/xc_private.c     | 12 ++++++++++++
>  tools/libxc/xc_private.h     |  2 ++
>  5 files changed, 33 insertions(+), 16 deletions(-)
> 
> diff --git a/tools/libxc/xc_core_arm.c b/tools/libxc/xc_core_arm.c
> index 16508e7..846bc6c 100644
> --- a/tools/libxc/xc_core_arm.c
> +++ b/tools/libxc/xc_core_arm.c
> @@ -30,12 +30,6 @@ xc_core_arch_gpfn_may_present(struct xc_core_arch_context 
> *arch_ctxt,
>      return 0;
>  }
>  
> -
> -static int nr_gpfns(xc_interface *xch, domid_t domid)
> -{
> -    return xc_domain_maximum_gpfn(xch, domid) + 1;
> -}
> -
>  int
>  xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info)
>  {
> @@ -48,9 +42,12 @@ xc_core_arch_memory_map_get(xc_interface *xch, struct 
> xc_core_arch_context *unus
>                              xc_core_memory_map_t **mapp,
>                              unsigned int *nr_entries)
>  {
> -    unsigned long p2m_size = nr_gpfns(xch, info->domid);
> +    unsigned long p2m_size = 0;
>      xc_core_memory_map_t *map;
>  
> +    if ( xc_nr_gpfns(xch, info->domid, &p2m_size) < 0 )
> +        return -1;
> +
>      map = malloc(sizeof(*map));
>      if ( map == NULL )
>      {
> diff --git a/tools/libxc/xc_core_x86.c b/tools/libxc/xc_core_x86.c
> index d8846f1..2f5ffea 100644
> --- a/tools/libxc/xc_core_x86.c
> +++ b/tools/libxc/xc_core_x86.c
> @@ -35,12 +35,6 @@ xc_core_arch_gpfn_may_present(struct xc_core_arch_context 
> *arch_ctxt,
>      return 1;
>  }
>  
> -
> -static int nr_gpfns(xc_interface *xch, domid_t domid)
> -{
> -    return xc_domain_maximum_gpfn(xch, domid) + 1;
> -}
> -
>  int
>  xc_core_arch_auto_translated_physmap(const xc_dominfo_t *info)
>  {
> @@ -53,9 +47,12 @@ xc_core_arch_memory_map_get(xc_interface *xch, struct 
> xc_core_arch_context *unus
>                              xc_core_memory_map_t **mapp,
>                              unsigned int *nr_entries)
>  {
> -    unsigned long p2m_size = nr_gpfns(xch, info->domid);
> +    unsigned long p2m_size = 0;
>      xc_core_memory_map_t *map;
>  
> +    if ( xc_nr_gpfns(xch, info->domid, &p2m_size) < 0 )
> +        return -1;
> +
>      map = malloc(sizeof(*map));
>      if ( map == NULL )
>      {
> @@ -88,7 +85,12 @@ xc_core_arch_map_p2m_rw(xc_interface *xch, struct 
> domain_info_context *dinfo, xc
>      int err;
>      int i;
>  
> -    dinfo->p2m_size = nr_gpfns(xch, info->domid);
> +    if ( xc_nr_gpfns(xch, info->domid, &dinfo->p2m_size) < 0 )
> +    {
> +        ERROR("Could not get maximum GPFN!");
> +        goto out;
> +    }
> +
>      if ( dinfo->p2m_size < info->nr_pages  )
>      {
>          ERROR("p2m_size < nr_pages -1 (%lx < %lx", dinfo->p2m_size, 
> info->nr_pages - 1);
> diff --git a/tools/libxc/xc_domain_save.c b/tools/libxc/xc_domain_save.c
> index 254fdb3..b410273 100644
> --- a/tools/libxc/xc_domain_save.c
> +++ b/tools/libxc/xc_domain_save.c
> @@ -939,7 +939,11 @@ int xc_domain_save(xc_interface *xch, int io_fd, 
> uint32_t dom, uint32_t max_iter
>      }
>  
>      /* Get the size of the P2M table */
> -    dinfo->p2m_size = xc_domain_maximum_gpfn(xch, dom) + 1;
> +    if ( xc_nr_gpfns(xch, dom, &dinfo->p2m_size) < 0 )
> +    {
> +        ERROR("Could not get maximum GPFN!");
> +        goto out;
> +    }
>  
>      if ( dinfo->p2m_size > ~XEN_DOMCTL_PFINFO_LTAB_MASK )
>      {
> diff --git a/tools/libxc/xc_private.c b/tools/libxc/xc_private.c
> index 0735e23..0eb49ee 100644
> --- a/tools/libxc/xc_private.c
> +++ b/tools/libxc/xc_private.c
> @@ -540,6 +540,18 @@ long xc_maximum_ram_page(xc_interface *xch)
>      return do_memory_op(xch, XENMEM_maximum_ram_page, NULL, 0);
>  }
>  
> +int xc_nr_gpfns(xc_interface *xch, domid_t domid, unsigned long *gpfns)
> +{
> +    int rc = xc_domain_maximum_gpfn(xch, domid);
> +
> +    if ( rc >= 0 )
> +    {
> +        *gpfns = rc + 1;
> +        rc = 0;
> +    }
> +    return rc;
> +}
> +
>  long long xc_domain_get_cpu_usage( xc_interface *xch, domid_t domid, int 
> vcpu )
>  {
>      DECLARE_DOMCTL;
> diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
> index 45b8644..4b7f001 100644
> --- a/tools/libxc/xc_private.h
> +++ b/tools/libxc/xc_private.h
> @@ -364,6 +364,8 @@ static inline int do_multicall_op(xc_interface *xch,
>  
>  int do_memory_op(xc_interface *xch, int cmd, void *arg, size_t len);
>  
> +int xc_nr_gpfns(xc_interface *xch, domid_t domid, unsigned long *gpfns);
> +
>  void *xc_map_foreign_ranges(xc_interface *xch, uint32_t dom,
>                              size_t size, int prot, size_t chunksize,
>                              privcmd_mmap_entry_t entries[], int nentries);
> -- 
> 2.1.0
> 
> > 
> > Ian.
> > 

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