All of lore.kernel.org
 help / color / mirror / Atom feed
From: Julien Grall <julien@xen.org>
To: xen-devel@lists.xenproject.org
Cc: julien@xen.org, Julien Grall <jgrall@amazon.com>,
	Ian Jackson <iwj@xenproject.org>, Wei Liu <wl@xen.org>,
	Juergen Gross <jgross@suse.com>
Subject: [PATCH] tools/xenstored: Fix off-by-one in dump_state_nodes()
Date: Thu, 29 Jul 2021 10:34:20 +0100	[thread overview]
Message-ID: <20210729093420.14092-1-julien@xen.org> (raw)

From: Julien Grall <jgrall@amazon.com>

The maximum path length supported by Xenstored protocol is
XENSTORE_ABS_PATH_MAX (i.e 3072). This doesn't take into account the
NUL at the end of the path.

However, the code to dump the nodes will allocate a buffer
of XENSTORE_ABS_PATH. As a result it may not be possible to live-update
if there is a node name of XENSTORE_ABS_PATH.

Fix it by allocating a buffer of XENSTORE_ABS_PATH_MAX + 1 characters.

Take the opportunity to pass the max length of the buffer as a
parameter of dump_state_node_tree(). This will be clearer that the
check in the function is linked to the allocation in dump_state_nodes().

Signed-off-by: Julien Grall <jgrall@amazon.com>

---

This was spotted when backporting Live-Update to 4.11 because the
commit 924bf8c793 "tools/xenstore: rework path length check" is
not present. On the latest upstream, this is looks more a latent bug
because I didn't manage to create such large node.

(4.11)

42sh# xenstore-write $(python -c "print('/' + 'A' * 3071)") ""
42sh# xenstore-control live-update /usr/local/sbin/xenstored
Starting live update failed:
Dump node path length error
---
 tools/xenstore/xenstored_core.c | 11 ++++++-----
 1 file changed, 6 insertions(+), 5 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 16c856730c55..0d4c73d6e20c 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2574,7 +2574,8 @@ const char *dump_state_node_perms(FILE *fp, const struct xs_permissions *perms,
 	return NULL;
 }
 
-static const char *dump_state_node_tree(FILE *fp, char *path)
+static const char *dump_state_node_tree(FILE *fp, char *path,
+					unsigned int path_max_len)
 {
 	unsigned int pathlen, childlen, p = 0;
 	struct xs_state_record_header head;
@@ -2642,10 +2643,10 @@ static const char *dump_state_node_tree(FILE *fp, char *path)
 	}
 	while (p < hdr->childlen) {
 		childlen = strlen(child) + 1;
-		if (pathlen + childlen > XENSTORE_ABS_PATH_MAX)
+		if (pathlen + childlen > path_max_len)
 			return "Dump node path length error";
 		strcpy(path + pathlen, child);
-		ret = dump_state_node_tree(fp, path);
+		ret = dump_state_node_tree(fp, path, path_max_len);
 		if (ret)
 			return ret;
 		p += childlen;
@@ -2661,13 +2662,13 @@ const char *dump_state_nodes(FILE *fp, const void *ctx)
 {
 	char *path;
 
-	path = talloc_size(ctx, XENSTORE_ABS_PATH_MAX);
+	path = talloc_size(ctx, XENSTORE_ABS_PATH_MAX + 1);
 	if (!path)
 		return "Path buffer allocation error";
 
 	strcpy(path, "/");
 
-	return dump_state_node_tree(fp, path);
+	return dump_state_node_tree(fp, path, XENSTORE_ABS_PATH_MAX + 1);
 }
 
 void read_state_global(const void *ctx, const void *state)
-- 
2.17.1



         reply	other threads:[~2021-07-29  9:34 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-29 11:06 [PATCH] tools/xenstored: Propagate correctly the error message from lu_start() Julien Grall
2021-07-30  7:07 ` Juergen Gross
2021-07-29  9:34   ` Julien Grall [this message]
2021-07-30  7:02     ` [PATCH] tools/xenstored: Fix off-by-one in dump_state_nodes() Juergen Gross
2021-07-30 10:04       ` [PATCH] tools/xenstored: Fix off-by-one in dump_state_nodes() [and 3 more messages] Ian Jackson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210729093420.14092-1-julien@xen.org \
    --to=julien@xen.org \
    --cc=iwj@xenproject.org \
    --cc=jgrall@amazon.com \
    --cc=jgross@suse.com \
    --cc=wl@xen.org \
    --cc=xen-devel@lists.xenproject.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.