Libguestfs - Nov 2015 - [PATCH] inspector: --xpath: Copy node to new document (RHBZ#1281577).

'virt-inspector --xpath' can segfault.

When run under valgrind, it shows this error:

==2254== Invalid free() / delete / delete[] / realloc()
==2254==    at 0x4C29D6A: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2254==    by 0x53BA198: xmlFreeNodeList (tree.c:3690)
==2254==    by 0x53B9F65: xmlFreeDoc (tree.c:1247)
==2254==    by 0x405BFA: do_xpath (inspector.c:808)
==2254==    by 0x405BFA: main (inspector.c:250)
==2254==  Address 0x1030a037 is 311 bytes inside a block of size 1,048
alloc'd
==2254==    at 0x4C28C50: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==2254==    by 0x545DE86: xmlDictAddString.isra.0 (dict.c:270)
==2254==    by 0x545E961: xmlDictLookup (dict.c:923)
==2254==    by 0x539C6DC: xmlDetectSAX2 (parser.c:1067)
==2254==    by 0x53B0B92: xmlParseDocument (parser.c:10725)
==2254==    by 0x53B1276: xmlDoRead (parser.c:15295)
==2254==    by 0x40587D: do_xpath (inspector.c:772)
==2254==    by 0x40587D: main (inspector.c:250)

The cause appears to be that when copying the matching node(s) found
by the xpath expression, we have to copy them into the new document
(using xmlDocCopyNode instead of xmlCopyNode).

This bug has existed since this functionality was originally added in
commit d1ee71782ace98a11c5aabaf1f9fd5f601e08367.
---
 inspector/inspector.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/inspector/inspector.c b/inspector/inspector.c
index d0013ed..dd5be44 100644
--- a/inspector/inspector.c
+++ b/inspector/inspector.c
@@ -811,7 +811,7 @@ do_xpath (const char *query)
                  guestfs_int_program_name);
         exit (EXIT_FAILURE);
       }
-      wrnode = xmlCopyNode (nodes->nodeTab[i], 1);
+      wrnode = xmlDocCopyNode (nodes->nodeTab[i], wrdoc, 1);
       if (wrnode == NULL) {
         fprintf (stderr, _("%s: xmlCopyNode failed\n"),
                  guestfs_int_program_name);
-- 
2.5.0

On Thursday 12 November 2015 22:43:06 Richard W.M. Jones
wrote:
> 'virt-inspector --xpath' can segfault.
> 
> When run under valgrind, it shows this error:
> 
> ==2254== Invalid free() / delete / delete[] / realloc()
> ==2254==    at 0x4C29D6A: free (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==2254==    by 0x53BA198: xmlFreeNodeList (tree.c:3690)
> ==2254==    by 0x53B9F65: xmlFreeDoc (tree.c:1247)
> ==2254==    by 0x405BFA: do_xpath (inspector.c:808)
> ==2254==    by 0x405BFA: main (inspector.c:250)
> ==2254==  Address 0x1030a037 is 311 bytes inside a block of size 1,048
alloc'd
> ==2254==    at 0x4C28C50: malloc (in
/usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
> ==2254==    by 0x545DE86: xmlDictAddString.isra.0 (dict.c:270)
> ==2254==    by 0x545E961: xmlDictLookup (dict.c:923)
> ==2254==    by 0x539C6DC: xmlDetectSAX2 (parser.c:1067)
> ==2254==    by 0x53B0B92: xmlParseDocument (parser.c:10725)
> ==2254==    by 0x53B1276: xmlDoRead (parser.c:15295)
> ==2254==    by 0x40587D: do_xpath (inspector.c:772)
> ==2254==    by 0x40587D: main (inspector.c:250)
> 
> The cause appears to be that when copying the matching node(s) found
> by the xpath expression, we have to copy them into the new document
> (using xmlDocCopyNode instead of xmlCopyNode).
> 
> This bug has existed since this functionality was originally added in
> commit d1ee71782ace98a11c5aabaf1f9fd5f601e08367.
> ---
LGTM.

Thanks,
-- 
Pino Toscano