xen-devel.lists.xenproject.org archive mirror
 help / color / mirror / Atom feed
* [PATCH]: fix crash in various tools by permitting xs_*() with NULL path
@ 2010-07-20 15:55 Gianni Tedesco
  2010-07-20 16:27 ` Ian Jackson
  0 siblings, 1 reply; 9+ messages in thread
From: Gianni Tedesco @ 2010-07-20 15:55 UTC (permalink / raw)
  To: Xen Devel

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@citrix.com>

 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);

^ permalink raw reply	[flat|nested] 9+ messages in thread
end of thread, other threads:[~2010-07-26  9:45 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-07-20 15:55 [PATCH]: fix crash in various tools by permitting xs_*() with NULL path Gianni Tedesco
2010-07-20 16:27 ` Ian Jackson
2010-07-20 17:02   ` Gianni Tedesco
2010-07-26  9:45     ` Paolo Bonzini
2010-07-20 17:16   ` Gianni Tedesco
2010-07-20 17:32   ` [PATCH]: fix segfault in xl destroy when xenstore has been fowled up Gianni Tedesco
2010-07-21 13:48     ` Stefano Stabellini
2010-07-21 13:59       ` Ian Jackson
2010-07-21  8:41   ` [PATCH]: fix crash in various tools by permitting xs_*() with NULL path 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).