WARNING - OLD ARCHIVES

This is an archived copy of the Xen.org mailing list, which we have preserved to ensure that existing links to archives are not broken. The live archive, which contains the latest emails, can be found at http://lists.xen.org/
   
 
 
Xen 
 
Home Products Support Community News
 
   
 

xen-devel

[Xen-devel] [PATCH] xenstore size limits

I wrote ("[PATCH] docs/misc/xenstore.txt minor fixes"):
> I'm going to follow this up with another patch about size limits for
> xenstore messages.

The patch below:
 * Documents the existing 4kby size limit on xenstore message payloads
 * Causes xs.c in libxenstore to fail locally rather than violating
   said limit (which is good because xenstored kills the client
   connection if it's exceeded).
 * Introduces some limits on path lengths in xenstored.  I trust
   no-one is using path lengths >2kby.  This is good because currently
   a domain client can create a 4kby relative path that the dom0 tools
   cannot access since they'd have to specify the somewhat longer
   absolute path.
 * Removes uses of the host's PATH_MAX (!)

Signed-off-by: Ian Jackson <ian.jackson@xxxxxxxxxxxxx>

diff -r 190e490154e9 docs/misc/xenstore.txt
--- a/docs/misc/xenstore.txt    Wed Dec 12 17:06:48 2007 +0000
+++ b/docs/misc/xenstore.txt    Thu Dec 13 11:46:39 2007 +0000
@@ -38,7 +38,9 @@ the four punctuation characters -/_@ (hy
 the four punctuation characters -/_@ (hyphen slash underscore atsign).
 @ should be avoided except to specify special watches (see below).
 Doubled slashes and trailing slashes (except to specify the root) are
-forbidden.  The empty path is also forbidden.
+forbidden.  The empty path is also forbidden.  Paths longer than 3072
+bytes are forbidden; clients specifying relative paths should keep
+them to within 2048 bytes.  (See XENSTORE_*_PATH_MAX in xs_wire.h.)
 
 
 Communication with xenstore is via either sockets, or event channel
@@ -55,6 +57,20 @@ order and must use req_id (and tx_id, if
 order and must use req_id (and tx_id, if applicable) to match up
 replies to requests.  (The current implementation always replies to
 requests in the order received but this should not be relied on.)
+
+The payload length (len field of the header) is limited to 4096
+(XENSTORE_PAYLOAD_MAX) in both directions.  If a client exceeds the
+limit, its xenstored connection will be immediately killed by
+xenstored, which is usually catastrophic from the client's point of
+view.  Clients (particularly domains, which cannot just reconnect)
+should avoid this.
+
+Existing clients do not always contain defences against overly long
+payloads.  Increasing xenstored's limit is therefore difficult; it
+would require negotiation with the client, and obviously would make
+parts of xenstore inaccessible to some clients.  In any case passing
+bulk data through xenstore is not recommended as the performance
+properties are poor.
 
 
 ---------- Xenstore protocol details - introduction ----------
diff -r 190e490154e9 tools/xenstore/xenstored_core.c
--- a/tools/xenstore/xenstored_core.c   Wed Dec 12 17:06:48 2007 +0000
+++ b/tools/xenstore/xenstored_core.c   Thu Dec 13 11:46:39 2007 +0000
@@ -672,6 +672,9 @@ bool is_valid_nodename(const char *node)
        if (strstr(node, "//"))
                return false;
 
+       if (strlen(node) > XENSTORE_ABS_PATH_MAX)
+               return false;
+
        return valid_chars(node);
 }
 
@@ -1281,7 +1284,7 @@ static void handle_input(struct connecti
                if (in->used != sizeof(in->hdr))
                        return;
 
-               if (in->hdr.msg.len > PATH_MAX) {
+               if (in->hdr.msg.len > XENSTORE_PAYLOAD_MAX) {
                        syslog(LOG_ERR, "Client tried to feed us %i",
                               in->hdr.msg.len);
                        goto bad_client;
diff -r 190e490154e9 tools/xenstore/xenstored_watch.c
--- a/tools/xenstore/xenstored_watch.c  Wed Dec 12 17:06:48 2007 +0000
+++ b/tools/xenstore/xenstored_watch.c  Thu Dec 13 11:46:39 2007 +0000
@@ -125,6 +125,10 @@ void do_watch(struct connection *conn, s
 
        if (strstarts(vec[0], "@")) {
                relative = false;
+               if (strlen(vec[0]) > XENSTORE_REL_PATH_MAX) {
+                       send_error(conn, EINVAL);
+                       return;
+               }
                /* check if valid event */
        } else {
                relative = !strstarts(vec[0], "/");
diff -r 190e490154e9 tools/xenstore/xs.c
--- a/tools/xenstore/xs.c       Wed Dec 12 17:06:48 2007 +0000
+++ b/tools/xenstore/xs.c       Thu Dec 13 11:46:39 2007 +0000
@@ -319,6 +319,11 @@ static void *xs_talkv(struct xs_handle *
        for (i = 0; i < num_vecs; i++)
                msg.len += iovec[i].iov_len;
 
+       if (msg.len > XENSTORE_PAYLOAD_MAX) {
+               errno = E2BIG;
+               return 0;
+       }
+
        ignorepipe.sa_handler = SIG_IGN;
        sigemptyset(&ignorepipe.sa_mask);
        ignorepipe.sa_flags = 0;
diff -r 190e490154e9 tools/xenstore/xsls.c
--- a/tools/xenstore/xsls.c     Wed Dec 12 17:06:48 2007 +0000
+++ b/tools/xenstore/xsls.c     Thu Dec 13 11:46:39 2007 +0000
@@ -8,7 +8,7 @@
 #include <sys/ioctl.h>
 #include <termios.h>
 
-#define STRING_MAX PATH_MAX
+#define STRING_MAX XENSTORE_ABS_PATH_MAX+1024
 static int max_width = 80;
 static int desired_width = 60;
 static int show_whole_path = 0;
diff -r 190e490154e9 xen/include/public/io/xs_wire.h
--- a/xen/include/public/io/xs_wire.h   Wed Dec 12 17:06:48 2007 +0000
+++ b/xen/include/public/io/xs_wire.h   Thu Dec 13 11:46:39 2007 +0000
@@ -108,6 +108,13 @@ struct xenstore_domain_interface {
     XENSTORE_RING_IDX rsp_cons, rsp_prod;
 };
 
+#define XENSTORE_PAYLOAD_MAX 4096
+  /* Violating this is very bad.  See docs/misc/xenstore.txt. */
+
+#define XENSTORE_ABS_PATH_MAX 3072
+#define XENSTORE_REL_PATH_MAX 2048
+  /* Violating these just gets you an error back */
+
 #endif /* _XS_WIRE_H */
 
 /*
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel
<Prev in Thread] Current Thread [Next in Thread>