Re: [Xen-devel] [PATCH]: fix crash in various tools by permitting xs_*() with NULL path

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

On Tue, 2010-07-20 at 17:27 +0100, Ian Jackson wrote:
> Gianni Tedesco writes ("[Xen-devel] [PATCH]: fix crash in various tools by 
> permitting xs_*() with NULL path"):
> > Many tools generate xenstore paths and then perform operations on those
> > paths without checking for NULL.
> 
> Do they ?  Those tools are buggy.
> 
> > While strictly this may be considered a bug in the tools it makes sense
> > to consider making these no-ops as a convenience measure.
> 
> I think this is fine for things like deletion but not otherwise.  So I
> think in the case of libxenstore only xs_rm should be modified like
> this.

I may be defeating my own argument but here is an alternative (let's
forget strdup() out of memory case and let it crash and burn):

8<---------------------
Fix segfault in xl destroy when xenstore has been fowled up

libxl_dirname() returns NULL if a string is passed to it which doesn't
look like a xenstore key. During xl destroy libxl_dirname is used to
generate an xs path in order to remove device backend keys. If a
nonsense key has been put in there resulting in NULL backend path this
will crash xl.

diff -r 1eea12f66951 tools/libxl/libxl_device.c
--- a/tools/libxl/libxl_device.c        Tue Jul 20 17:14:43 2010 +0100
+++ b/tools/libxl/libxl_device.c        Tue Jul 20 18:05:19 2010 +0100
@@ -344,7 +344,8 @@ int libxl_devices_destroy(struct libxl_c
     }
     for (i = 0; i < n; i++) {
         flexarray_get(toremove, i, (void**) &path);
-        xs_rm(clone.xsh, XBT_NULL, path);
+        if ( path )
+            xs_rm(clone.xsh, XBT_NULL, path);
     }
     flexarray_free(toremove);
     libxl_ctx_free(&clone);



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel