Konrad Rzeszutek Wilk
2013-Sep-10 15:08 UTC
[PATCH] xenstat: Fix buffer over-run with new_domains being negative.
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 <konrad.wilk@oracle.com>
---
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)
--
1.7.7.6
Andrew Cooper
2013-Sep-10 16:10 UTC
Re: [PATCH] xenstat: Fix buffer over-run with new_domains being negative.
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 <konrad.wilk@oracle.com>Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>> --- > 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)
Ian Campbell
2013-Sep-13 12:32 UTC
Re: [PATCH] xenstat: Fix buffer over-run with new_domains being negative.
On Tue, 2013-09-10 at 17:10 +0100, Andrew Cooper wrote:> 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 <konrad.wilk@oracle.com> > > Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>Applied.