Re: [Xen-devel] [PATCH] xenstat: Fix buffer over-run with new_domains being negative.

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 10/09/13 16:08, Konrad Rzeszutek Wilk wrote:
> Coverity identified this as:
> CID 1055740 Out-of-bounds read - "In xenstat_get_node:
> Out-of-bounds read from a buffer (CWE-125)"
>
> And sure enough, if xc_domain_getinfolist returns us -1, we will
> try to use it later on in the for (i = 0; i < new_domains; ..)
> loop.
>
> CC: ian.campbell@xxxxxxxxxx
> Signed-off-by: Konrad Rzeszutek Wilk <konrad.wilk@xxxxxxxxxx>

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

> ---
>  tools/xenstat/libxenstat/src/xenstat.c |   14 +++++++++-----
>  1 files changed, 9 insertions(+), 5 deletions(-)
>
> diff --git a/tools/xenstat/libxenstat/src/xenstat.c 
> b/tools/xenstat/libxenstat/src/xenstat.c
> index 104655d..e5facb8 100644
> --- a/tools/xenstat/libxenstat/src/xenstat.c
> +++ b/tools/xenstat/libxenstat/src/xenstat.c
> @@ -208,15 +208,15 @@ xenstat_node *xenstat_get_node(xenstat_handle * handle, 
> unsigned int flags)
>                                                   node->num_domains, 
>                                                   DOMAIN_CHUNK_SIZE, 
>                                                   domaininfo);
> +             if (new_domains < 0)
> +                     goto err;
>  
>               tmp = realloc(node->domains,
>                             (node->num_domains + new_domains)
>                             * sizeof(xenstat_domain));
> -             if (tmp == NULL) {
> -                     free(node->domains);
> -                     free(node);
> -                     return NULL;
> -             }
> +             if (tmp == NULL)
> +                     goto err;
> +
>               node->domains = tmp;
>  
>               domain = node->domains + node->num_domains;
> @@ -280,6 +280,10 @@ xenstat_node *xenstat_get_node(xenstat_handle * handle, 
> unsigned int flags)
>       }
>  
>       return node;
> +err:
> +     free(node->domains);
> +     free(node);
> +     return NULL;
>  }
>  
>  void xenstat_free_node(xenstat_node * node)


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