* [PATCH] xl: do not leak cpupool names
@ 2012-09-06 11:46 Ian Campbell
2012-09-06 13:43 ` Juergen Gross
0 siblings, 1 reply; 3+ messages in thread
From: Ian Campbell @ 2012-09-06 11:46 UTC (permalink / raw)
To: xen-devel; +Cc: Ian.Jackson, Juergen Gross
# HG changeset patch
# User Ian Campbell <ian.campbell@citrix.com>
# 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 <ian.campbell@citrix.com>
---
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 <unknown>)\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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] xl: do not leak cpupool names
2012-09-06 11:46 [PATCH] xl: do not leak cpupool names Ian Campbell
@ 2012-09-06 13:43 ` Juergen Gross
2012-09-14 9:06 ` Ian Campbell
0 siblings, 1 reply; 3+ messages in thread
From: Juergen Gross @ 2012-09-06 13:43 UTC (permalink / raw)
To: Ian Campbell; +Cc: Ian.Jackson, xen-devel
Am 06.09.2012 13:46, schrieb Ian Campbell:
> # HG changeset patch
> # User Ian Campbell<ian.campbell@citrix.com>
> # 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<ian.campbell@citrix.com>
Acked-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
> ---
>
> 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<unknown>)\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
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] xl: do not leak cpupool names
2012-09-06 13:43 ` Juergen Gross
@ 2012-09-14 9:06 ` Ian Campbell
0 siblings, 0 replies; 3+ messages in thread
From: Ian Campbell @ 2012-09-14 9:06 UTC (permalink / raw)
To: Juergen Gross; +Cc: Ian Jackson, xen-devel@lists.xen.org
On Thu, 2012-09-06 at 14:43 +0100, Juergen Gross wrote:
> Am 06.09.2012 13:46, schrieb Ian Campbell:
> > # HG changeset patch
> > # User Ian Campbell<ian.campbell@citrix.com>
> > # 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<ian.campbell@citrix.com>
>
> Acked-by: Juergen Gross<juergen.gross@ts.fujitsu.com>
Applied, thanks.
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2012-09-14 9:06 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2012-09-06 11:46 [PATCH] xl: do not leak cpupool names Ian Campbell
2012-09-06 13:43 ` Juergen Gross
2012-09-14 9:06 ` Ian Campbell
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).