diff options
Diffstat (limited to '0083-tools-xenstore-create_node-Don-t-defer-work-to-undo-.patch')
| -rw-r--r-- | 0083-tools-xenstore-create_node-Don-t-defer-work-to-undo-.patch | 120 |
1 files changed, 120 insertions, 0 deletions
diff --git a/0083-tools-xenstore-create_node-Don-t-defer-work-to-undo-.patch b/0083-tools-xenstore-create_node-Don-t-defer-work-to-undo-.patch new file mode 100644 index 0000000..5204b3f --- /dev/null +++ b/0083-tools-xenstore-create_node-Don-t-defer-work-to-undo-.patch @@ -0,0 +1,120 @@ +From ee03d9b56e6141422b4ef2444f93cf2e88e6a26c Mon Sep 17 00:00:00 2001 +From: Julien Grall <jgrall@amazon.com> +Date: Tue, 13 Sep 2022 07:35:06 +0200 +Subject: [PATCH 083/126] tools/xenstore: create_node: Don't defer work to undo + any changes on failure + +XSA-115 extended destroy_node() to update the node accounting for the +connection. The implementation is assuming the connection is the parent +of the node, however all the nodes are allocated using a separate context +(see process_message()). This will result to crash (or corrupt) xenstored +as the pointer is wrongly used. + +In case of an error, any changes to the database or update to the +accounting will now be reverted in create_node() by calling directly +destroy_node(). This has the nice advantage to remove the loop to unset +the destructors in case of success. + +Take the opportunity to free the nodes right now as they are not +going to be reachable (the function returns NULL) and are just wasting +resources. + +This is XSA-414 / CVE-2022-42309. + +Fixes: 0bfb2101f243 ("tools/xenstore: fix node accounting after failed node creation") +Signed-off-by: Julien Grall <jgrall@amazon.com> +Reviewed-by: Juergen Gross <jgross@suse.com> +(cherry picked from commit 1cd3cc7ea27cda7640a8d895e09617b61c265697) +--- + tools/xenstore/xenstored_core.c | 47 ++++++++++++++++++++++----------- + 1 file changed, 32 insertions(+), 15 deletions(-) + +diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c +index 9172dd767140..a00c49e404a1 100644 +--- a/tools/xenstore/xenstored_core.c ++++ b/tools/xenstore/xenstored_core.c +@@ -1054,9 +1054,8 @@ nomem: + return NULL; + } + +-static int destroy_node(void *_node) ++static int destroy_node(struct connection *conn, struct node *node) + { +- struct node *node = _node; + TDB_DATA key; + + if (streq(node->name, "/")) +@@ -1065,7 +1064,7 @@ static int destroy_node(void *_node) + set_tdb_key(node->name, &key); + tdb_delete(tdb_ctx, key); + +- domain_entry_dec(talloc_parent(node), node); ++ domain_entry_dec(conn, node); + + return 0; + } +@@ -1074,7 +1073,8 @@ static struct node *create_node(struct connection *conn, const void *ctx, + const char *name, + void *data, unsigned int datalen) + { +- struct node *node, *i; ++ struct node *node, *i, *j; ++ int ret; + + node = construct_node(conn, ctx, name); + if (!node) +@@ -1096,23 +1096,40 @@ static struct node *create_node(struct connection *conn, const void *ctx, + /* i->parent is set for each new node, so check quota. */ + if (i->parent && + domain_entry(conn) >= quota_nb_entry_per_domain) { +- errno = ENOSPC; +- return NULL; ++ ret = ENOSPC; ++ goto err; + } +- if (write_node(conn, i, false)) +- return NULL; + +- /* Account for new node, set destructor for error case. */ +- if (i->parent) { ++ ret = write_node(conn, i, false); ++ if (ret) ++ goto err; ++ ++ /* Account for new node */ ++ if (i->parent) + domain_entry_inc(conn, i); +- talloc_set_destructor(i, destroy_node); +- } + } + +- /* OK, now remove destructors so they stay around */ +- for (i = node; i->parent; i = i->parent) +- talloc_set_destructor(i, NULL); + return node; ++ ++err: ++ /* ++ * We failed to update TDB for some of the nodes. Undo any work that ++ * have already been done. ++ */ ++ for (j = node; j != i; j = j->parent) ++ destroy_node(conn, j); ++ ++ /* We don't need to keep the nodes around, so free them. */ ++ i = node; ++ while (i) { ++ j = i; ++ i = i->parent; ++ talloc_free(j); ++ } ++ ++ errno = ret; ++ ++ return NULL; + } + + /* path, data... */ +-- +2.37.4 + |