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)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 1cc11d0b1c30d45f8d1e1d719bf3704265ea70a2..5bb7b8521324d4e59e04cc186947c494d4359c5f 100644 (file)
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1233,21 +1233,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)
 {
@@ -1516,69 +1501,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;
@@ -1586,9 +1561,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;
 }
 
 
@@ -1623,7 +1610,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;