[xen master] tools/xenstore: use treewalk for check_store()

[patchbot@xxxxxxx]

commit a07cc0ec60612f414bedf2bafb26ec38d2602e95
Author:     Juergen Gross <jgross@xxxxxxxx>
AuthorDate: Tue Sep 13 07:35:12 2022 +0200
Commit:     Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CommitDate: Tue Nov 1 13:05:44 2022 +0000

    tools/xenstore: use treewalk for check_store()
    
    Instead of doing an open tree walk using call recursion, use
    walk_node_tree() when checking the store for inconsistencies.
    
    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@xxxxxxxx>
    Reviewed-by: Julien Grall <jgrall@xxxxxxxxxx>
---
 tools/xenstore/xenstored_core.c | 106 ++++++++++------------------------------
 1 file changed, 26 insertions(+), 80 deletions(-)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index b3596c7eeb..b0889186b6 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -2423,18 +2423,6 @@ int remember_string(struct hashtable *hash, const char 
*str)
        return hashtable_insert(hash, k, (void *)1);
 }
 
-static int rm_child_entry(struct node *node, size_t off, size_t len)
-{
-       if (!recovery)
-               return off;
-
-       if (remove_child_entry(NULL, node, off))
-               log("check_store: child entry could not be removed from '%s'",
-                   node->name);
-
-       return off - len - 1;
-}
-
 /**
  * A node has a children field that names the children of the node, separated
  * by NULs.  We check whether there are entries in there that are duplicated
@@ -2448,70 +2436,29 @@ static int rm_child_entry(struct node *node, size_t 
off, size_t len)
  * As we go, we record each node in the given reachable hashtable.  These
  * entries will be used later in clean_store.
  */
-static int check_store_(const char *name, struct hashtable *reachable)
+static int check_store_step(const void *ctx, struct connection *conn,
+                           struct node *node, void *arg)
 {
-       struct node *node = read_node(NULL, name, name);
-       int ret = 0;
-
-       if (node) {
-               size_t i = 0;
-
-               if (!remember_string(reachable, name)) {
-                       log("check_store: ENOMEM");
-                       return ENOMEM;
-               }
-
-               while (i < node->childlen && !ret) {
-                       struct node *childnode = NULL;
-                       size_t childlen = strlen(node->children + i);
-                       char *childname = child_name(NULL, node->name,
-                                                    node->children + i);
-
-                       if (!childname) {
-                               log("check_store: ENOMEM");
-                               ret = ENOMEM;
-                               break;
-                       }
-
-                       if (hashtable_search(reachable, childname)) {
-                               log("check_store: '%s' is duplicated!",
-                                   childname);
-                               i = rm_child_entry(node, i, childlen);
-                               goto next;
-                       }
+       struct hashtable *reachable = arg;
 
-                       childnode = read_node(NULL, childname, childname);
-
-                       if (childnode) {
-                               ret = check_store_(childname, reachable);
-                       } else if (errno != ENOMEM) {
-                               log("check_store: No child '%s' found!\n",
-                                   childname);
-                               i = rm_child_entry(node, i, childlen);
-                       } else {
-                               log("check_store: ENOMEM");
-                               ret = ENOMEM;
-                       }
+       if (hashtable_search(reachable, (void *)node->name)) {
+               log("check_store: '%s' is duplicated!", node->name);
+               return recovery ? WALK_TREE_RM_CHILDENTRY
+                               : WALK_TREE_SKIP_CHILDREN;
+       }
 
- next:
-                       talloc_free(childnode);
-                       talloc_free(childname);
-                       i += childlen + 1;
-               }
+       if (!remember_string(reachable, node->name))
+               return WALK_TREE_ERROR_STOP;
 
-               talloc_free(node);
-       } else if (errno != ENOMEM) {
-               /* Impossible, because no database should ever be without the
-                  root, and otherwise, we've just checked in our caller
-                  (which made a recursive call to get here). */
+       return WALK_TREE_OK;
+}
 
-               log("check_store: No child '%s' found: impossible!", name);
-       } else {
-               log("check_store: ENOMEM");
-               ret = ENOMEM;
-       }
+static int check_store_enoent(const void *ctx, struct connection *conn,
+                             struct node *parent, char *name, void *arg)
+{
+       log("check_store: node '%s' not found", name);
 
-       return ret;
+       return recovery ? WALK_TREE_RM_CHILDENTRY : WALK_TREE_OK;
 }
 
 
@@ -2560,29 +2507,28 @@ static void clean_store(struct hashtable *reachable)
 
 void check_store(void)
 {
-       char *root = talloc_strdup(NULL, "/");
        struct hashtable *reachable;
+       struct walk_funcs walkfuncs = {
+               .enter = check_store_step,
+               .enoent = check_store_enoent,
+       };
 
-       if (!root) {
-               log("check_store: ENOMEM");
-               return;
-       }
        reachable = create_hashtable(16, hash_from_key_fn, keys_equal_fn);
        if (!reachable) {
                log("check_store: ENOMEM");
-               goto out_root;
+               return;
        }
 
        log("Checking store ...");
-       if (!check_store_(root, reachable) &&
-           !check_transactions(reachable))
+       if (walk_node_tree(NULL, NULL, "/", &walkfuncs, reachable)) {
+               if (errno == ENOMEM)
+                       log("check_store: ENOMEM");
+       } else if (!check_transactions(reachable))
                clean_store(reachable);
        log("Checking store complete.");
 
        hashtable_destroy(reachable, 0 /* Don't free values (they are all
                                          (void *)1) */);
- out_root:
-       talloc_free(root);
 }
 
 
--
generated by git-patchbot for /home/xen/git/xen.git#master