From mboxrd@z Thu Jan 1 00:00:00 1970 From: Andrew Cooper Subject: Re: [PATCH] xenstat: Fix buffer over-run with new_domains being negative. Date: Tue, 10 Sep 2013 17:10:41 +0100 Message-ID: <522F4481.9080209@citrix.com> References: <1378825710-9563-1-git-send-email-konrad.wilk@oracle.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Return-path: Received: from mail6.bemta3.messagelabs.com ([195.245.230.39]) by lists.xen.org with esmtp (Exim 4.72) (envelope-from ) id 1VJQWy-0007Us-MX for xen-devel@lists.xenproject.org; Tue, 10 Sep 2013 16:11:08 +0000 In-Reply-To: <1378825710-9563-1-git-send-email-konrad.wilk@oracle.com> List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Konrad Rzeszutek Wilk Cc: xen-devel@lists.xenproject.org, ian.campbell@citrix.com List-Id: xen-devel@lists.xenproject.org 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@citrix.com > Signed-off-by: Konrad Rzeszutek Wilk Reviewed-by: Andrew Cooper > --- > 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)