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
|
From 9ead5845034c04a5c6e04d9b069d9c13141f4f33 Mon Sep 17 00:00:00 2001
From: Juergen Gross <jgross@suse.com>
Date: Tue, 13 Sep 2022 07:35:12 +0200
Subject: [PATCH 116/126] tools/xenstore: use treewalk for deleting nodes
Instead of doing an open tree walk using call recursion, use
walk_node_tree() when deleting a sub-tree of nodes.
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>
Acked-by: Julien Grall <jgrall@amazon.com>
(cherry picked from commit ea16962053a6849a6e7cada549ba7f8c586d85c6)
---
tools/xenstore/xenstored_core.c | 99 ++++++++++++++-------------------
1 file changed, 43 insertions(+), 56 deletions(-)
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index ed8bc9b02ed2..9576411757fa 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1300,21 +1300,6 @@ static int do_read(const void *ctx, struct connection *conn,
return 0;
}
-static void delete_node_single(struct connection *conn, struct node *node)
-{
- TDB_DATA key;
-
- if (access_node(conn, node, NODE_ACCESS_DELETE, &key))
- return;
-
- if (do_tdb_delete(conn, &key, &node->acc) != 0) {
- corrupt(conn, "Could not delete '%s'", node->name);
- return;
- }
-
- domain_entry_dec(conn, node);
-}
-
/* Must not be / */
static char *basename(const char *name)
{
@@ -1585,69 +1570,59 @@ static int remove_child_entry(struct connection *conn, struct node *node,
return write_node(conn, node, true);
}
-static void delete_child(struct connection *conn,
- struct node *node, const char *childname)
+static int delete_child(struct connection *conn,
+ struct node *node, const char *childname)
{
unsigned int i;
for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) {
if (streq(node->children+i, childname)) {
- if (remove_child_entry(conn, node, i))
- corrupt(conn, "Can't update parent node '%s'",
- node->name);
- return;
+ errno = remove_child_entry(conn, node, i) ? EIO : 0;
+ return errno;
}
}
corrupt(conn, "Can't find child '%s' in %s", childname, node->name);
+
+ errno = EIO;
+ return errno;
}
-static int delete_node(struct connection *conn, const void *ctx,
- struct node *parent, struct node *node, bool watch_exact)
+static int delnode_sub(const void *ctx, struct connection *conn,
+ struct node *node, void *arg)
{
- char *name;
+ const char *root = arg;
+ bool watch_exact;
+ int ret;
+ TDB_DATA key;
- /* Delete children. */
- while (node->childlen) {
- struct node *child;
+ /* Any error here will probably be repeated for all following calls. */
+ ret = access_node(conn, node, NODE_ACCESS_DELETE, &key);
+ if (ret > 0)
+ return WALK_TREE_SUCCESS_STOP;
- name = talloc_asprintf(node, "%s/%s", node->name,
- node->children);
- child = name ? read_node(conn, node, name) : NULL;
- if (child) {
- if (delete_node(conn, ctx, node, child, true))
- return errno;
- } else {
- trace("delete_node: Error deleting child '%s/%s'!\n",
- node->name, node->children);
- /* Quit deleting. */
- errno = ENOMEM;
- return errno;
- }
- talloc_free(name);
- }
+ /* In case of error stop the walk. */
+ if (!ret && do_tdb_delete(conn, &key, &node->acc))
+ return WALK_TREE_SUCCESS_STOP;
/*
* Fire the watches now, when we can still see the node permissions.
* This fine as we are single threaded and the next possible read will
* be handled only after the node has been really removed.
- */
+ */
+ watch_exact = strcmp(root, node->name);
fire_watches(conn, ctx, node->name, node, watch_exact, NULL);
- delete_node_single(conn, node);
- delete_child(conn, parent, basename(node->name));
- talloc_free(node);
- return 0;
+ domain_entry_dec(conn, node);
+
+ return WALK_TREE_RM_CHILDENTRY;
}
-static int _rm(struct connection *conn, const void *ctx, struct node *node,
- const char *name)
+static int _rm(struct connection *conn, const void *ctx, const char *name)
{
- /*
- * Deleting node by node, so the result is always consistent even in
- * case of a failure.
- */
struct node *parent;
char *parentname = get_parent(ctx, name);
+ struct walk_funcs walkfuncs = { .exit = delnode_sub };
+ int ret;
if (!parentname)
return errno;
@@ -1655,9 +1630,21 @@ static int _rm(struct connection *conn, const void *ctx, struct node *node,
parent = read_node(conn, ctx, parentname);
if (!parent)
return read_node_can_propagate_errno() ? errno : EINVAL;
- node->parent = parent;
- return delete_node(conn, ctx, parent, node, false);
+ ret = walk_node_tree(ctx, conn, name, &walkfuncs, (void *)name);
+ if (ret < 0) {
+ if (ret == WALK_TREE_ERROR_STOP) {
+ corrupt(conn, "error when deleting sub-nodes of %s\n",
+ name);
+ errno = EIO;
+ }
+ return errno;
+ }
+
+ if (delete_child(conn, parent, basename(name)))
+ return errno;
+
+ return 0;
}
@@ -1694,7 +1681,7 @@ static int do_rm(const void *ctx, struct connection *conn,
if (streq(name, "/"))
return EINVAL;
- ret = _rm(conn, ctx, node, name);
+ ret = _rm(conn, ctx, name);
if (ret)
return ret;
--
2.37.4
|
GitWeb