summaryrefslogtreecommitdiff
blob: 6271169ef2e5688b9e85965c63a89e5d45023672 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
From 84674f206778e9b3d8d67c6c76aa8094a262d5ec Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Tue, 13 Sep 2022 07:35:12 +0200
Subject: [PATCH 117/126] 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.

Signed-off-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit 297ac246a5d8ed656b349641288f3402dcc0251e)
---
 tools/xenstore/xenstored_core.c   | 127 ++++++++++++------------------
 tools/xenstore/xenstored_core.h   |   3 +-
 tools/xenstore/xenstored_domain.c |   2 +-
 3 files changed, 54 insertions(+), 78 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 9576411757fa..e8cdfeef50c7 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2990,132 +2990,109 @@ const char *dump_state_buffered_data(FILE *fp, const struct connection *c,
 	return NULL;
 }
 
-const char *dump_state_node_perms(FILE *fp, struct xs_state_node *sn,
-				  const struct xs_permissions *perms,
+const char *dump_state_node_perms(FILE *fp, const struct xs_permissions *perms,
 				  unsigned int n_perms)
 {
 	unsigned int p;
 
 	for (p = 0; p < n_perms; p++) {
+		struct xs_state_node_perm sp;
+
 		switch ((int)perms[p].perms & ~XS_PERM_IGNORE) {
 		case XS_PERM_READ:
-			sn->perms[p].access = XS_STATE_NODE_PERM_READ;
+			sp.access = XS_STATE_NODE_PERM_READ;
 			break;
 		case XS_PERM_WRITE:
-			sn->perms[p].access = XS_STATE_NODE_PERM_WRITE;
+			sp.access = XS_STATE_NODE_PERM_WRITE;
 			break;
 		case XS_PERM_READ | XS_PERM_WRITE:
-			sn->perms[p].access = XS_STATE_NODE_PERM_BOTH;
+			sp.access = XS_STATE_NODE_PERM_BOTH;
 			break;
 		default:
-			sn->perms[p].access = XS_STATE_NODE_PERM_NONE;
+			sp.access = XS_STATE_NODE_PERM_NONE;
 			break;
 		}
-		sn->perms[p].flags = (perms[p].perms & XS_PERM_IGNORE)
+		sp.flags = (perms[p].perms & XS_PERM_IGNORE)
 				     ? XS_STATE_NODE_PERM_IGNORE : 0;
-		sn->perms[p].domid = perms[p].id;
-	}
+		sp.domid = perms[p].id;
 
-	if (fwrite(sn->perms, sizeof(*sn->perms), n_perms, fp) != n_perms)
-		return "Dump node permissions error";
+		if (fwrite(&sp, sizeof(sp), 1, fp) != 1)
+			return "Dump node permissions error";
+	}
 
 	return NULL;
 }
 
-static const char *dump_state_node_tree(FILE *fp, char *path)
+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, &sn, 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(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 (fwrite(node->name, pathlen, 1, fp) != 1)
+		return dump_state_node_err(data, "Dump node path 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 > XENSTORE_ABS_PATH_MAX)
-			return "Dump node path length error";
-		strcpy(path + pathlen, child);
-		ret = dump_state_node_tree(fp, path);
-		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;
+	struct dump_node_data data = {
+		.fp = fp,
+		.err = "Dump node walk error"
+	};
+	struct walk_funcs walkfuncs = { .enter = dump_state_node };
 
-	path = talloc_size(ctx, XENSTORE_ABS_PATH_MAX);
-	if (!path)
-		return "Path buffer allocation error";
+	if (walk_node_tree(ctx, NULL, "/", &walkfuncs, &data))
+		return data.err;
 
-	strcpy(path, "/");
-
-	return dump_state_node_tree(fp, path);
+	return NULL;
 }
 
 void read_state_global(const void *ctx, const void *state)
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index f0fd8c352857..3190494bbeb5 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -326,8 +326,7 @@ const char *dump_state_buffered_data(FILE *fp, const struct connection *c,
 				     const struct connection *conn,
 				     struct xs_state_connection *sc);
 const char *dump_state_nodes(FILE *fp, const void *ctx);
-const char *dump_state_node_perms(FILE *fp, struct xs_state_node *sn,
-				  const struct xs_permissions *perms,
+const char *dump_state_node_perms(FILE *fp, const struct xs_permissions *perms,
 				  unsigned int n_perms);
 
 void read_state_global(const void *ctx, const void *state);
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index 8b503c2dfe07..a91cc75ab59b 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1449,7 +1449,7 @@ static const char *dump_state_special_node(FILE *fp, const char *name,
 	if (fwrite(&sn, sizeof(sn), 1, fp) != 1)
 		return "Dump special node error";
 
-	ret = dump_state_node_perms(fp, &sn, perms->p, perms->num);
+	ret = dump_state_node_perms(fp, perms->p, perms->num);
 	if (ret)
 		return ret;
 
-- 
2.37.4