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

[Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>]

Many tools generate xenstore paths and then perform operations on those
paths without checking for NULL. The problem with this is that xs_single
and xs_talkv use iovecs where len is set to strlen(NULL) + 1 leading to
a deref.

While strictly this may be considered a bug in the tools it makes sense
to consider making these no-ops as a convenience measure.

If the iov_len for NULL is set to 0 then this causes xenstored not to
respond and for the client to hang indefinitely. For this reason the
entry to each affected library function is modified to check for NULL.

I have left xs_watch and xs_unwatch as before since there is no
reasonable no-op implementation that I can think of.

Signed-off-by: Gianni Tedesco <gianni.tedesco@xxxxxxxxxx>

 xenstore/xs.c  |   18 ++++++++++++++++++
 xenstore/xs.h  |    4 ++++
 3 files changed, 23 insertions(+), 1 deletion(-)


diff -r 108ee7b37ac4 tools/xenstore/xs.c
--- a/tools/xenstore/xs.c       Tue Jul 20 15:01:15 2010 +0100
+++ b/tools/xenstore/xs.c       Tue Jul 20 16:44:43 2010 +0100
@@ -474,6 +474,9 @@
        char *strings, *p, **ret;
        unsigned int len;
 
+    if ( NULL == path )
+        return NULL;
+
        strings = xs_single(h, t, XS_DIRECTORY, path, &len);
        if (!strings)
                return NULL;
@@ -503,6 +506,8 @@
 void *xs_read(struct xs_handle *h, xs_transaction_t t,
              const char *path, unsigned int *len)
 {
+    if ( NULL == path )
+        return NULL;
        return xs_single(h, t, XS_READ, path, len);
 }
 
@@ -514,6 +519,9 @@
 {
        struct iovec iovec[2];
 
+    if ( NULL == path )
+        return true;
+
        iovec[0].iov_base = (void *)path;
        iovec[0].iov_len = strlen(path) + 1;
        iovec[1].iov_base = (void *)data;
@@ -529,6 +537,8 @@
 bool xs_mkdir(struct xs_handle *h, xs_transaction_t t,
              const char *path)
 {
+    if ( NULL == path )
+        return true;
        return xs_bool(xs_single(h, t, XS_MKDIR, path, NULL));
 }
 
@@ -538,6 +548,8 @@
 bool xs_rm(struct xs_handle *h, xs_transaction_t t,
           const char *path)
 {
+    if ( NULL == path )
+        return true;
        return xs_bool(xs_single(h, t, XS_RM, path, NULL));
 }
 
@@ -552,6 +564,9 @@
        unsigned int len;
        struct xs_permissions *ret;
 
+    if ( NULL == path )
+        return NULL;
+
        strings = xs_single(h, t, XS_GET_PERMS, path, &len);
        if (!strings)
                return NULL;
@@ -587,6 +602,9 @@
        unsigned int i;
        struct iovec iov[1+num_perms];
 
+    if ( NULL == path )
+        return true;
+
        iov[0].iov_base = (void *)path;
        iov[0].iov_len = strlen(path) + 1;
        
diff -r 108ee7b37ac4 tools/xenstore/xs.h
--- a/tools/xenstore/xs.h       Tue Jul 20 15:01:15 2010 +0100
+++ b/tools/xenstore/xs.h       Tue Jul 20 16:44:43 2010 +0100
@@ -110,6 +110,8 @@
  * When the node (or any child) changes, fd will become readable.
  * Token is returned when watch is read, to allow matching.
  * Returns false on failure.
+ *
+ * path must be non-NULL
  */
 bool xs_watch(struct xs_handle *h, const char *path, const char *token);
 
@@ -124,6 +126,8 @@
 
 /* Remove a watch on a node: implicitly acks any outstanding watch.
  * Returns false on failure (no watch on that node).
+ *
+ * path must be non-NULL
  */
 bool xs_unwatch(struct xs_handle *h, const char *path, const char *token);
 



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