[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen master] tools/xenstore: don't let remove_child_entry() call corrupt()
commit 0c00c51f3bc8206c7f9cf87d014650157bee2bf4 Author: Juergen Gross <jgross@xxxxxxxx> AuthorDate: Tue Sep 13 07:35:11 2022 +0200 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Tue Nov 1 13:05:44 2022 +0000 tools/xenstore: don't let remove_child_entry() call corrupt() In case of write_node() returning an error, remove_child_entry() will call corrupt() today. This could result in an endless recursion, as remove_child_entry() is called by corrupt(), too: corrupt() check_store() check_store_() remove_child_entry() Fix that by letting remove_child_entry() return an error instead and let the caller decide what to do. 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 | 36 ++++++++++++++++++++---------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c index cb52f68d4d..5642917e67 100644 --- a/tools/xenstore/xenstored_core.c +++ b/tools/xenstore/xenstored_core.c @@ -1604,15 +1604,15 @@ static void memdel(void *mem, unsigned off, unsigned len, unsigned total) memmove(mem + off, mem + off + len, total - off - len); } -static void remove_child_entry(struct connection *conn, struct node *node, - size_t offset) +static int remove_child_entry(struct connection *conn, struct node *node, + size_t offset) { size_t childlen = strlen(node->children + offset); memdel(node->children, offset, childlen + 1, node->childlen); node->childlen -= childlen + 1; - if (write_node(conn, node, true)) - corrupt(conn, "Can't update parent node '%s'", node->name); + + return write_node(conn, node, true); } static void delete_child(struct connection *conn, @@ -1622,7 +1622,9 @@ static void delete_child(struct connection *conn, for (i = 0; i < node->childlen; i += strlen(node->children+i) + 1) { if (streq(node->children+i, childname)) { - remove_child_entry(conn, node, i); + if (remove_child_entry(conn, node, i)) + corrupt(conn, "Can't update parent node '%s'", + node->name); return; } } @@ -2304,6 +2306,17 @@ 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 @@ -2356,12 +2369,7 @@ static int check_store_(const char *name, struct hashtable *reachable) if (hashtable_search(children, childname)) { log("check_store: '%s' is duplicated!", childname); - - if (recovery) { - remove_child_entry(NULL, node, - i); - i -= childlen + 1; - } + i = rm_child_entry(node, i, childlen); } else { if (!remember_string(children, @@ -2378,11 +2386,7 @@ static int check_store_(const char *name, struct hashtable *reachable) } else if (errno != ENOMEM) { log("check_store: No child '%s' found!\n", childname); - - if (recovery) { - remove_child_entry(NULL, node, i); - i -= childlen + 1; - } + i = rm_child_entry(node, i, childlen); } else { log("check_store: ENOMEM"); ret = ENOMEM; -- generated by git-patchbot for /home/xen/git/xen.git#master
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |