Blob Blame History Raw
From 1ee281b18b52bec87335ea64ee74cc159e63d036 Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Tue, 13 Sep 2022 07:35:12 +0200
Subject: tools/xenstore: use treewalk for creating node records

Instead of doing an open tree walk using call recursion, use
walk_node_tree() when creating the node records during a live update.

This will reduce code size and avoid many nesting levels of function
calls which could potentially exhaust the stack.

This is part of XSA-418 / CVE-2022-42321.

Reported-by: Julien Grall <dvrabel@amazon.co.uk>
Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 58fb651542ec..05d349778bb4 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -3120,101 +3120,76 @@ 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,
-					unsigned int path_max_len)
+struct dump_node_data {
+	FILE *fp;
+	const char *err;
+};
+
+static int dump_state_node_err(struct dump_node_data *data, const char *err)
 {
-	unsigned int pathlen, childlen, p = 0;
+	data->err = err;
+	return WALK_TREE_ERROR_STOP;
+}
+
+static int dump_state_node(const void *ctx, struct connection *conn,
+			   struct node *node, void *arg)
+{
+	struct dump_node_data *data = arg;
+	FILE *fp = data->fp;
+	unsigned int pathlen;
 	struct xs_state_record_header head;
 	struct xs_state_node sn;
-	TDB_DATA key, data;
-	const struct xs_tdb_record_hdr *hdr;
-	const char *child;
 	const char *ret;
 
-	pathlen = strlen(path) + 1;
-
-	set_tdb_key(path, &key);
-	data = tdb_fetch(tdb_ctx, key);
-	if (data.dptr == NULL)
-		return "Error reading node";
-
-	/* Clean up in case of failure. */
-	talloc_steal(path, data.dptr);
-
-	hdr = (void *)data.dptr;
+	pathlen = strlen(node->name) + 1;
 
 	head.type = XS_STATE_TYPE_NODE;
 	head.length = sizeof(sn);
 	sn.conn_id = 0;
 	sn.ta_id = 0;
 	sn.ta_access = 0;
-	sn.perm_n = hdr->num_perms;
+	sn.perm_n = node->perms.num;
 	sn.path_len = pathlen;
-	sn.data_len = hdr->datalen;
-	head.length += hdr->num_perms * sizeof(*sn.perms);
+	sn.data_len = node->datalen;
+	head.length += node->perms.num * sizeof(*sn.perms);
 	head.length += pathlen;
-	head.length += hdr->datalen;
+	head.length += node->datalen;
 	head.length = ROUNDUP(head.length, 3);
 
 	if (fwrite(&head, sizeof(head), 1, fp) != 1)
-		return "Dump node state error";
+		return dump_state_node_err(data, "Dump node head error");
 	if (fwrite(&sn, sizeof(sn), 1, fp) != 1)
-		return "Dump node state error";
+		return dump_state_node_err(data, "Dump node state error");
 
-	ret = dump_state_node_perms(fp, hdr->perms, hdr->num_perms);
+	ret = dump_state_node_perms(fp, node->perms.p, node->perms.num);
 	if (ret)
-		return ret;
+		return dump_state_node_err(data, ret);
+
+	if (fwrite(node->name, pathlen, 1, fp) != 1)
+		return dump_state_node_err(data, "Dump node path error");
 
-	if (fwrite(path, pathlen, 1, fp) != 1)
-		return "Dump node path error";
-	if (hdr->datalen &&
-	    fwrite(hdr->perms + hdr->num_perms, hdr->datalen, 1, fp) != 1)
-		return "Dump node data error";
+	if (node->datalen && fwrite(node->data, node->datalen, 1, fp) != 1)
+		return dump_state_node_err(data, "Dump node data error");
 
 	ret = dump_state_align(fp);
 	if (ret)
-		return ret;
+		return dump_state_node_err(data, ret);
 
-	child = (char *)(hdr->perms + hdr->num_perms) + hdr->datalen;
-
-	/*
-	 * Use path for constructing children paths.
-	 * As we don't write out nodes without having written their parent
-	 * already we will never clobber a part of the path we'll need later.
-	 */
-	pathlen--;
-	if (path[pathlen - 1] != '/') {
-		path[pathlen] = '/';
-		pathlen++;
-	}
-	while (p < hdr->childlen) {
-		childlen = strlen(child) + 1;
-		if (pathlen + childlen > path_max_len)
-			return "Dump node path length error";
-		strcpy(path + pathlen, child);
-		ret = dump_state_node_tree(fp, path, path_max_len);
-		if (ret)
-			return ret;
-		p += childlen;
-		child += childlen;
-	}
-
-	talloc_free(data.dptr);
-
-	return NULL;
+	return WALK_TREE_OK;
 }
 
 const char *dump_state_nodes(FILE *fp, const void *ctx)
 {
-	char *path;
-
-	path = talloc_size(ctx, XENSTORE_ABS_PATH_MAX + 1);
-	if (!path)
-		return "Path buffer allocation error";
+	struct dump_node_data data = {
+		.fp = fp,
+		.err = "Dump node walk error"
+	};
+	struct walk_funcs walkfuncs = { .enter = dump_state_node };
 
-	strcpy(path, "/");
+	if (walk_node_tree(ctx, NULL, "/", &walkfuncs, &data))
+		return data.err;
 
-	return dump_state_node_tree(fp, path, XENSTORE_ABS_PATH_MAX + 1);
+	return NULL;
 }
 
 void read_state_global(const void *ctx, const void *state)