From mboxrd@z Thu Jan 1 00:00:00 1970 From: Boris Ostrovsky Subject: Re: [PATCH] tools: Fix wild memory allocations from c/s 250f0b4 and 85d78b4 Date: Mon, 18 May 2015 10:00:58 -0400 Message-ID: <5559F09A.8020505@oracle.com> References: <1431953844-32035-1-git-send-email-andrew.cooper3@citrix.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: <1431953844-32035-1-git-send-email-andrew.cooper3@citrix.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: Andrew Cooper , Xen-devel Cc: Wei Liu , Ian Jackson , Ian Campbell List-Id: xen-devel@lists.xenproject.org On 05/18/2015 08:57 AM, Andrew Cooper wrote: > These changesets cause the respective libxc functions to unconditonally > dereference their max_cpus/nodes parameters as part of initial memory > allocations. It will fail at obtaining the correct number of cpus/nodes from > Xen, as the guest handles will not be NULL. > > Signed-off-by: Andrew Cooper > CC: Ian Campbell > CC: Ian Jackson > CC: Wei Liu > CC: Boris Ostrovsky > > --- > Spotted by XenServers Coverity run. > --- > tools/libxl/libxl.c | 4 ++-- > tools/misc/xenpm.c | 4 ++-- > tools/python/xen/lowlevel/xc/xc.c | 4 ++-- > 3 files changed, 6 insertions(+), 6 deletions(-) xenpm bug is already fixed (commit b315cd9cce5b6da7ca89b2d7bad3fb01e7716044 n the staging tree). I am not sure I understand why Coverity complains about other spots. For example, in libxl_get_cpu_topology() num_cpus can be left uninitialized only if xc_cputopoinfo(ctx->xch, &num_cpus, NULL) fails, in which case we go to 'GC_FREE; return ret;', so it's not ever used. -boris > > diff --git a/tools/libxl/libxl.c b/tools/libxl/libxl.c > index a6eb2df..295877b 100644 > --- a/tools/libxl/libxl.c > +++ b/tools/libxl/libxl.c > @@ -5105,7 +5105,7 @@ libxl_cputopology *libxl_get_cpu_topology(libxl_ctx *ctx, int *nb_cpu_out) > xc_cputopo_t *cputopo; > libxl_cputopology *ret = NULL; > int i; > - unsigned num_cpus; > + unsigned num_cpus = 0; > > /* Setting buffer to NULL makes the call return number of CPUs */ > if (xc_cputopoinfo(ctx->xch, &num_cpus, NULL)) > @@ -5191,7 +5191,7 @@ libxl_numainfo *libxl_get_numainfo(libxl_ctx *ctx, int *nr) > uint32_t *distance; > libxl_numainfo *ret = NULL; > int i, j; > - unsigned num_nodes; > + unsigned num_nodes = 0; > > if (xc_numainfo(ctx->xch, &num_nodes, NULL, NULL)) { > LOGE(ERROR, "Unable to determine number of nodes"); > diff --git a/tools/misc/xenpm.c b/tools/misc/xenpm.c > index fe2c001..2f9bd8e 100644 > --- a/tools/misc/xenpm.c > +++ b/tools/misc/xenpm.c > @@ -356,7 +356,7 @@ static void signal_int_handler(int signo) > struct timeval tv; > int cx_cap = 0, px_cap = 0; > xc_cputopo_t *cputopo = NULL; > - unsigned max_cpus; > + unsigned max_cpus = 0; > > if ( xc_cputopoinfo(xc_handle, &max_cpus, NULL) != 0 ) > { > @@ -961,7 +961,7 @@ void scaling_governor_func(int argc, char *argv[]) > void cpu_topology_func(int argc, char *argv[]) > { > xc_cputopo_t *cputopo = NULL; > - unsigned max_cpus; > + unsigned max_cpus = 0; > int i, rc; > > if ( xc_cputopoinfo(xc_handle, &max_cpus, NULL) != 0 ) > diff --git a/tools/python/xen/lowlevel/xc/xc.c b/tools/python/xen/lowlevel/xc/xc.c > index fbd93db..c77e15b 100644 > --- a/tools/python/xen/lowlevel/xc/xc.c > +++ b/tools/python/xen/lowlevel/xc/xc.c > @@ -1221,7 +1221,7 @@ static PyObject *pyxc_getcpuinfo(XcObject *self, PyObject *args, PyObject *kwds) > static PyObject *pyxc_topologyinfo(XcObject *self) > { > xc_cputopo_t *cputopo = NULL; > - unsigned i, num_cpus; > + unsigned i, num_cpus = 0; > PyObject *ret_obj = NULL; > PyObject *cpu_to_core_obj, *cpu_to_socket_obj, *cpu_to_node_obj; > > @@ -1293,7 +1293,7 @@ static PyObject *pyxc_topologyinfo(XcObject *self) > > static PyObject *pyxc_numainfo(XcObject *self) > { > - unsigned i, j, num_nodes; > + unsigned i, j, num_nodes = 0; > uint64_t free_heap; > PyObject *ret_obj = NULL, *node_to_node_dist_list_obj; > PyObject *node_to_memsize_obj, *node_to_memfree_obj;