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)

diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 9576411757fa85e440c6992bf2248061aaef8cd6..e8cdfeef50c7b0e71ade19d6c8793f73abef0808 100644 (file)
--- 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)
+{
+       data->err = err;
+       return WALK_TREE_ERROR_STOP;
+}
+
+static int dump_state_node(const void *ctx, struct connection *conn,
+                          struct node *node, void *arg)
 {
-       unsigned int pathlen, childlen, p = 0;
+       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;
-
-       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 dump_state_node_err(data, ret);
 
-       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);
-       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);
+       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 f0fd8c3528573719d6ac3b534d9e54fcc3a2939c..3190494bbeb5781f892ab4bd60a6045e87829b4d 100644 (file)
--- 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 8b503c2dfe07d96b80283b35cdc0594f9e095e13..a91cc75ab59bb3a0e39be2faa1f98e70df1e5995 100644 (file)
--- 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;