[Libvir] [PATCH] Delete a harmful variable in xend_parse_sexp_desc()

Hi Xend_parse_sexp_desc() of xend_internal.c declares a variable named "ret" to buffer SXP. However, it actually uses "buf.content" not "ret" and allocates memory to "buf" when size of "ret" became insufficient. --> virBufferAdd(&buf, ...);, virBufferVSprintf(&buf, ...) So freeing "ret" fails, because "ret" refers to an address unlike "buf.contents" As a result, a segmentation fault occurs. This patch fixes so that xend_parse_sexp_desc() uses "buf.contents" not "ret". Signed-off-by: Masayuki Sunou <fj1826dm@aa.jp.fujitsu.com> Thanks, Masayuki Sunou. -------------------------------------------------------------------------------- Index: src/xend_internal.c =================================================================== RCS file: /data/cvs/libvirt/src/xend_internal.c,v retrieving revision 1.135 diff -u -p -r1.135 xend_internal.c --- src/xend_internal.c 21 Aug 2007 09:31:12 -0000 1.135 +++ src/xend_internal.c 27 Aug 2007 07:47:03 -0000 @@ -1346,7 +1346,6 @@ xend_parse_sexp_desc_os(virConnectPtr xe static char * xend_parse_sexp_desc(virConnectPtr conn, struct sexpr *root, int xendConfigVersion) { - char *ret; struct sexpr *cur, *node; const char *tmp; char *tty; @@ -1362,10 +1361,9 @@ xend_parse_sexp_desc(virConnectPtr conn, /* ERROR */ return (NULL); } - ret = malloc(4000); - if (ret == NULL) + buf.content = malloc(4000); + if (buf.content == NULL) return (NULL); - buf.content = ret; buf.size = 4000; buf.use = 0; @@ -1762,11 +1760,11 @@ xend_parse_sexp_desc(virConnectPtr conn, virBufferAdd(&buf, "</domain>\n", 10); buf.content[buf.use] = 0; - return (ret); + return (buf.content); error: - if (ret != NULL) - free(ret); + if (buf.content != NULL) + free(buf.content); return (NULL); } --------------------------------------------------------------------------------

On Mon, Aug 27, 2007 at 05:37:29PM +0900, Masayuki Sunou wrote:
Hi
Xend_parse_sexp_desc() of xend_internal.c declares a variable named "ret" to buffer SXP. However, it actually uses "buf.content" not "ret" and allocates memory to "buf" when size of "ret" became insufficient. --> virBufferAdd(&buf, ...);, virBufferVSprintf(&buf, ...) So freeing "ret" fails, because "ret" refers to an address unlike "buf.contents" As a result, a segmentation fault occurs.
This patch fixes so that xend_parse_sexp_desc() uses "buf.contents" not "ret".
Oh, right, that's fairly nasty, and will occur only if the size of the XML grow over 4kB ! I tried to look for other patterns like that in the code but hopefully that's the only place where this was done. Thanks a lot for the patch, applied and commited ! Daniel -- Red Hat Virtualization group http://redhat.com/virtualization/ Daniel Veillard | virtualization library http://libvirt.org/ veillard@redhat.com | libxml GNOME XML XSLT toolkit http://xmlsoft.org/ http://veillard.com/ | Rpmfind RPM search engine http://rpmfind.net/
participants (2)
-
Daniel Veillard
-
Masayuki Sunou