From mboxrd@z Thu Jan 1 00:00:00 1970 From: Juergen Gross Subject: Re: [PATCH] xl: do not leak cpupool names Date: Thu, 06 Sep 2012 15:43:05 +0200 Message-ID: <5048A869.6060002@ts.fujitsu.com> References: Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii"; Format="flowed" Content-Transfer-Encoding: 7bit Return-path: In-Reply-To: List-Unsubscribe: , List-Post: List-Help: List-Subscribe: , Sender: xen-devel-bounces@lists.xen.org Errors-To: xen-devel-bounces@lists.xen.org To: Ian Campbell Cc: Ian.Jackson@citrix.com, xen-devel@lists.xen.org List-Id: xen-devel@lists.xenproject.org Am 06.09.2012 13:46, schrieb Ian Campbell: > # HG changeset patch > # User Ian Campbell > # Date 1346931944 -3600 > # Node ID d4d7031eb68bf0ce9bc83901b699cf13a2c73c95 > # Parent 1db796c3c0b97bba222f20db543c227e313ec1be > xl: do not leak cpupool names. > > Valgrind reports: > ==3076== 7 bytes in 1 blocks are definitely lost in loss record 1 of 1 > ==3076== at 0x402458C: malloc (vg_replace_malloc.c:270) > ==3076== by 0x406F86D: libxl_cpupoolid_to_name (libxl_utils.c:102) > ==3076== by 0x8058742: parse_config_data (xl_cmdimpl.c:639) > ==3076== by 0x805BD56: create_domain (xl_cmdimpl.c:1838) > ==3076== by 0x805DAED: main_create (xl_cmdimpl.c:3903) > ==3076== by 0x804D39D: main (xl.c:285) > > And indeed there are several places where xl uses > libxl_cpupoolid_to_name as a boolean to test if the pool name is > valid and leaks the name if it is. Introduce an is_valid helper and > use that instead. > > Signed-off-by: Ian Campbell Acked-by: Juergen Gross > --- > > This leak is in xl not libxl so this is 4.3 material IMHO. > > diff -r 1db796c3c0b9 -r d4d7031eb68b tools/libxl/libxl_utils.c > --- a/tools/libxl/libxl_utils.c Thu Sep 06 12:24:44 2012 +0100 > +++ b/tools/libxl/libxl_utils.c Thu Sep 06 12:45:44 2012 +0100 > @@ -103,6 +103,17 @@ char *libxl_cpupoolid_to_name(libxl_ctx > return s; > } > > +/* This is a bit horrid but without xs_exists it seems like the only way. */ > +int libxl_cpupoolid_is_valid(libxl_ctx *ctx, uint32_t poolid) > +{ > + int ret; > + char *s = libxl_cpupoolid_to_name(ctx, poolid); > + > + ret = (s != NULL); > + free(s); > + return ret; > +} > + > char *libxl__cpupoolid_to_name(libxl__gc *gc, uint32_t poolid) > { > char *s = libxl_cpupoolid_to_name(libxl__gc_owner(gc), poolid); > diff -r 1db796c3c0b9 -r d4d7031eb68b tools/libxl/libxl_utils.h > --- a/tools/libxl/libxl_utils.h Thu Sep 06 12:24:44 2012 +0100 > +++ b/tools/libxl/libxl_utils.h Thu Sep 06 12:45:44 2012 +0100 > @@ -24,6 +24,7 @@ int libxl_name_to_domid(libxl_ctx *ctx, > char *libxl_domid_to_name(libxl_ctx *ctx, uint32_t domid); > int libxl_name_to_cpupoolid(libxl_ctx *ctx, const char *name, uint32_t *poolid); > char *libxl_cpupoolid_to_name(libxl_ctx *ctx, uint32_t poolid); > +int libxl_cpupoolid_is_valid(libxl_ctx *ctx, uint32_t poolid); > int libxl_get_stubdom_id(libxl_ctx *ctx, int guest_domid); > int libxl_is_stubdom(libxl_ctx *ctx, uint32_t domid, uint32_t *target_domid); > int libxl_create_logfile(libxl_ctx *ctx, const char *name, char **full_name); > diff -r 1db796c3c0b9 -r d4d7031eb68b tools/libxl/xl_cmdimpl.c > --- a/tools/libxl/xl_cmdimpl.c Thu Sep 06 12:24:44 2012 +0100 > +++ b/tools/libxl/xl_cmdimpl.c Thu Sep 06 12:45:44 2012 +0100 > @@ -636,7 +636,7 @@ static void parse_config_data(const char > c_info->poolid = -1; > cpupool_qualifier_to_cpupoolid(buf,&c_info->poolid, NULL); > } > - if (!libxl_cpupoolid_to_name(ctx, c_info->poolid)) { > + if (!libxl_cpupoolid_is_valid(ctx, c_info->poolid)) { > fprintf(stderr, "Illegal pool specified\n"); > exit(1); > } > @@ -4733,7 +4733,7 @@ static int sched_domain_output(libxl_sch > > if (cpupool) { > if (cpupool_qualifier_to_cpupoolid(cpupool,&poolid, NULL) || > - !libxl_cpupoolid_to_name(ctx, poolid)) { > + !libxl_cpupoolid_is_valid(ctx, poolid)) { > fprintf(stderr, "unknown cpupool \'%s\'\n", cpupool); > return -ERROR_FAIL; > } > @@ -4863,7 +4863,7 @@ int main_sched_credit(int argc, char **a > > if (cpupool) { > if (cpupool_qualifier_to_cpupoolid(cpupool,&poolid, NULL) || > - !libxl_cpupoolid_to_name(ctx, poolid)) { > + !libxl_cpupoolid_is_valid(ctx, poolid)) { > fprintf(stderr, "unknown cpupool \'%s\'\n", cpupool); > return -ERROR_FAIL; > } > @@ -6290,7 +6290,7 @@ int main_cpupooldestroy(int argc, char * > pool = argv[optind]; > > if (cpupool_qualifier_to_cpupoolid(pool,&poolid, NULL) || > - !libxl_cpupoolid_to_name(ctx, poolid)) { > + !libxl_cpupoolid_is_valid(ctx, poolid)) { > fprintf(stderr, "unknown cpupool \'%s\'\n", pool); > return -ERROR_FAIL; > } > @@ -6311,7 +6311,7 @@ int main_cpupoolrename(int argc, char ** > pool = argv[optind++]; > > if (cpupool_qualifier_to_cpupoolid(pool,&poolid, NULL) || > - !libxl_cpupoolid_to_name(ctx, poolid)) { > + !libxl_cpupoolid_is_valid(ctx, poolid)) { > fprintf(stderr, "unknown cpupool \'%s\'\n", pool); > return -ERROR_FAIL; > } > @@ -6348,7 +6348,7 @@ int main_cpupoolcpuadd(int argc, char ** > } > > if (cpupool_qualifier_to_cpupoolid(pool,&poolid, NULL) || > - !libxl_cpupoolid_to_name(ctx, poolid)) { > + !libxl_cpupoolid_is_valid(ctx, poolid)) { > fprintf(stderr, "unknown cpupool \'%s\'\n", pool); > return -ERROR_FAIL; > } > @@ -6392,7 +6392,7 @@ int main_cpupoolcpuremove(int argc, char > } > > if (cpupool_qualifier_to_cpupoolid(pool,&poolid, NULL) || > - !libxl_cpupoolid_to_name(ctx, poolid)) { > + !libxl_cpupoolid_is_valid(ctx, poolid)) { > fprintf(stderr, "unknown cpupool \'%s\'\n", pool); > return -ERROR_FAIL; > } > @@ -6435,7 +6435,7 @@ int main_cpupoolmigrate(int argc, char * > } > > if (cpupool_qualifier_to_cpupoolid(pool,&poolid, NULL) || > - !libxl_cpupoolid_to_name(ctx, poolid)) { > + !libxl_cpupoolid_is_valid(ctx, poolid)) { > fprintf(stderr, "unknown cpupool \'%s\'\n", pool); > return -ERROR_FAIL; > } > diff -r 1db796c3c0b9 -r d4d7031eb68b tools/libxl/xl_sxp.c > --- a/tools/libxl/xl_sxp.c Thu Sep 06 12:24:44 2012 +0100 > +++ b/tools/libxl/xl_sxp.c Thu Sep 06 12:45:44 2012 +0100 > @@ -37,6 +37,7 @@ void printf_info_sexp(int domid, libxl_d > > libxl_domain_create_info *c_info =&d_config->c_info; > libxl_domain_build_info *b_info =&d_config->b_info; > + char *pool; > > printf("(domain\n\t(domid %d)\n", domid); > printf("\t(create_info)\n"); > @@ -54,8 +55,10 @@ void printf_info_sexp(int domid, libxl_d > } else { > printf("\t(uuid)\n"); > } > - > - printf("\t(cpupool %s)\n", libxl_cpupoolid_to_name(ctx, c_info->poolid)); > + pool = libxl_cpupoolid_to_name(ctx, c_info->poolid); > + if (pool) > + printf("\t(cpupool %s)\n", pool); > + free(pool); > if (c_info->xsdata) > printf("\t(xsdata contains data)\n"); > else > > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xen.org > http://lists.xen.org/xen-devel > > -- Juergen Gross Principal Developer Operating Systems PBG PDG ES&S SWE OS6 Telephone: +49 (0) 89 3222 2967 Fujitsu Technology Solutions e-mail: juergen.gross@ts.fujitsu.com Domagkstr. 28 Internet: ts.fujitsu.com D-80807 Muenchen Company details: ts.fujitsu.com/imprint.html