From a4a0cd435a2d74fc8901cc7979cc37745389f96e Mon Sep 17 00:00:00 2001 From: Juergen Gross Date: Tue, 13 Sep 2022 07:35:12 +0200 Subject: 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. Reported-by: Julien Grall Signed-off-by: Juergen Gross Reviewed-by: Julien Grall 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) +{ + 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 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;