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
|
Home