Xen 4.15.4-pre-patchset-24.15.4-pre-patchset-2

Signed-off-by: Florian Schmaus <flow@gentoo.org>

1 files changed, 410 insertions, 0 deletions

diff --git a/0124-tools-xenstore-harden-transaction-finalization-again.patch b/0124-tools-xenstore-harden-transaction-finalization-again.patch
new file mode 100644
index 0000000..8279aeb
--- /dev/null
+++ b/0124-tools-xenstore-harden-transaction-finalization-again.patch
@@ -0,0 +1,410 @@
+From e818f4f0dabf83a6138cd77d7464495fab7bfc16 Mon Sep 17 00:00:00 2001
+From: Juergen Gross <jgross@suse.com>
+Date: Tue, 13 Sep 2022 07:35:14 +0200
+Subject: [PATCH 124/126] tools/xenstore: harden transaction finalization
+ against errors
+
+When finalizing a transaction, any error occurring after checking for
+conflicts will result in the transaction being performed only
+partially today. Additionally accounting data will not be updated at
+the end of the transaction, which might result in further problems
+later.
+
+Avoid those problems by multiple modifications:
+
+- free any transaction specific nodes which don't need to be committed
+  as they haven't been written during the transaction as soon as their
+  generation count has been verified, this will reduce the risk of
+  out-of-memory situations
+
+- store the transaction specific node name in struct accessed_node in
+  order to avoid the need to allocate additional memory for it when
+  finalizing the transaction
+
+- don't stop the transaction finalization when hitting an error
+  condition, but try to continue to handle all modified nodes
+
+- in case of a detected error do the accounting update as needed and
+  call the data base checking only after that
+
+- if writing a node in a transaction is failing (e.g. due to a failed
+  quota check), fail the transaction, as prior changes to struct
+  accessed_node can't easily be undone in that case
+
+This is part of XSA-421 / CVE-2022-42326.
+
+Signed-off-by: Juergen Gross <jgross@suse.com>
+Reviewed-by: Julien Grall <jgrall@amazon.com>
+Tested-by: Julien Grall <jgrall@amazon.com>
+(cherry picked from commit 2dd823ca7237e7fb90c890642d6a3b357a26fcff)
+---
+ tools/xenstore/xenstored_core.c        |  16 ++-
+ tools/xenstore/xenstored_transaction.c | 171 +++++++++++--------------
+ tools/xenstore/xenstored_transaction.h |   4 +-
+ 3 files changed, 92 insertions(+), 99 deletions(-)
+
+diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
+index 9ddbd934f794..3c008c8cd455 100644
+--- a/tools/xenstore/xenstored_core.c
++++ b/tools/xenstore/xenstored_core.c
+@@ -692,8 +692,7 @@ struct node *read_node(struct connection *conn, const void *ctx,
+ 		return NULL;
+ 	}
+ 
+-	if (transaction_prepend(conn, name, &key))
+-		return NULL;
++	transaction_prepend(conn, name, &key);
+ 
+ 	data = tdb_fetch(tdb_ctx, key);
+ 
+@@ -811,10 +810,21 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node,
+ static int write_node(struct connection *conn, struct node *node,
+ 		      bool no_quota_check)
+ {
++	int ret;
++
+ 	if (access_node(conn, node, NODE_ACCESS_WRITE, &node->key))
+ 		return errno;
+ 
+-	return write_node_raw(conn, &node->key, node, no_quota_check);
++	ret = write_node_raw(conn, &node->key, node, no_quota_check);
++	if (ret && conn && conn->transaction) {
++		/*
++		 * Reverting access_node() is hard, so just fail the
++		 * transaction.
++		 */
++		fail_transaction(conn->transaction);
++	}
++
++	return ret;
+ }
+ 
+ enum xs_perm_type perm_for_conn(struct connection *conn,
+diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
+index 7ffe21bb5285..ac854197cadb 100644
+--- a/tools/xenstore/xenstored_transaction.c
++++ b/tools/xenstore/xenstored_transaction.c
+@@ -114,7 +114,8 @@ struct accessed_node
+ 	struct list_head list;
+ 
+ 	/* The name of the node. */
+-	char *node;
++	char *trans_name;	/* Transaction specific name. */
++	char *node;		/* Main data base name. */
+ 
+ 	/* Generation count (or NO_GENERATION) for conflict checking. */
+ 	uint64_t generation;
+@@ -199,25 +200,20 @@ static char *transaction_get_node_name(void *ctx, struct transaction *trans,
+  * Prepend the transaction to name if node has been modified in the current
+  * transaction.
+  */
+-int transaction_prepend(struct connection *conn, const char *name,
+-			TDB_DATA *key)
++void transaction_prepend(struct connection *conn, const char *name,
++			 TDB_DATA *key)
+ {
+-	char *tdb_name;
++	struct accessed_node *i;
+ 
+-	if (!conn || !conn->transaction ||
+-	    !find_accessed_node(conn->transaction, name)) {
+-		set_tdb_key(name, key);
+-		return 0;
++	if (conn && conn->transaction) {
++		i = find_accessed_node(conn->transaction, name);
++		if (i) {
++			set_tdb_key(i->trans_name, key);
++			return;
++		}
+ 	}
+ 
+-	tdb_name = transaction_get_node_name(conn->transaction,
+-					     conn->transaction, name);
+-	if (!tdb_name)
+-		return errno;
+-
+-	set_tdb_key(tdb_name, key);
+-
+-	return 0;
++	set_tdb_key(name, key);
+ }
+ 
+ /*
+@@ -240,7 +236,6 @@ int access_node(struct connection *conn, struct node *node,
+ 	struct accessed_node *i = NULL;
+ 	struct transaction *trans;
+ 	TDB_DATA local_key;
+-	const char *trans_name = NULL;
+ 	int ret;
+ 	bool introduce = false;
+ 
+@@ -259,10 +254,6 @@ int access_node(struct connection *conn, struct node *node,
+ 
+ 	trans = conn->transaction;
+ 
+-	trans_name = transaction_get_node_name(node, trans, node->name);
+-	if (!trans_name)
+-		goto nomem;
+-
+ 	i = find_accessed_node(trans, node->name);
+ 	if (!i) {
+ 		if (trans->nodes >= quota_trans_nodes &&
+@@ -273,9 +264,10 @@ int access_node(struct connection *conn, struct node *node,
+ 		i = talloc_zero(trans, struct accessed_node);
+ 		if (!i)
+ 			goto nomem;
+-		i->node = talloc_strdup(i, node->name);
+-		if (!i->node)
++		i->trans_name = transaction_get_node_name(i, trans, node->name);
++		if (!i->trans_name)
+ 			goto nomem;
++		i->node = strchr(i->trans_name, '/') + 1;
+ 		if (node->generation != NO_GENERATION && node->perms.num) {
+ 			i->perms.p = talloc_array(i, struct xs_permissions,
+ 						  node->perms.num);
+@@ -302,7 +294,7 @@ int access_node(struct connection *conn, struct node *node,
+ 			i->generation = node->generation;
+ 			i->check_gen = true;
+ 			if (node->generation != NO_GENERATION) {
+-				set_tdb_key(trans_name, &local_key);
++				set_tdb_key(i->trans_name, &local_key);
+ 				ret = write_node_raw(conn, &local_key, node, true);
+ 				if (ret)
+ 					goto err;
+@@ -321,7 +313,7 @@ int access_node(struct connection *conn, struct node *node,
+ 		return -1;
+ 
+ 	if (key) {
+-		set_tdb_key(trans_name, key);
++		set_tdb_key(i->trans_name, key);
+ 		if (type == NODE_ACCESS_WRITE)
+ 			i->ta_node = true;
+ 		if (type == NODE_ACCESS_DELETE)
+@@ -333,7 +325,6 @@ int access_node(struct connection *conn, struct node *node,
+ nomem:
+ 	ret = ENOMEM;
+ err:
+-	talloc_free((void *)trans_name);
+ 	talloc_free(i);
+ 	trans->fail = true;
+ 	errno = ret;
+@@ -371,100 +362,90 @@ void queue_watches(struct connection *conn, const char *name, bool watch_exact)
+  * base.
+  */
+ static int finalize_transaction(struct connection *conn,
+-				struct transaction *trans)
++				struct transaction *trans, bool *is_corrupt)
+ {
+-	struct accessed_node *i;
++	struct accessed_node *i, *n;
+ 	TDB_DATA key, ta_key, data;
+ 	struct xs_tdb_record_hdr *hdr;
+ 	uint64_t gen;
+-	char *trans_name;
+-	int ret;
+ 
+-	list_for_each_entry(i, &trans->accessed, list) {
+-		if (!i->check_gen)
+-			continue;
++	list_for_each_entry_safe(i, n, &trans->accessed, list) {
++		if (i->check_gen) {
++			set_tdb_key(i->node, &key);
++			data = tdb_fetch(tdb_ctx, key);
++			hdr = (void *)data.dptr;
++			if (!data.dptr) {
++				if (tdb_error(tdb_ctx) != TDB_ERR_NOEXIST)
++					return EIO;
++				gen = NO_GENERATION;
++			} else
++				gen = hdr->generation;
++			talloc_free(data.dptr);
++			if (i->generation != gen)
++				return EAGAIN;
++		}
+ 
+-		set_tdb_key(i->node, &key);
+-		data = tdb_fetch(tdb_ctx, key);
+-		hdr = (void *)data.dptr;
+-		if (!data.dptr) {
+-			if (tdb_error(tdb_ctx) != TDB_ERR_NOEXIST)
+-				return EIO;
+-			gen = NO_GENERATION;
+-		} else
+-			gen = hdr->generation;
+-		talloc_free(data.dptr);
+-		if (i->generation != gen)
+-			return EAGAIN;
++		/* Entries for unmodified nodes can be removed early. */
++		if (!i->modified) {
++			if (i->ta_node) {
++				set_tdb_key(i->trans_name, &ta_key);
++				if (do_tdb_delete(conn, &ta_key, NULL))
++					return EIO;
++			}
++			list_del(&i->list);
++			talloc_free(i);
++		}
+ 	}
+ 
+ 	while ((i = list_top(&trans->accessed, struct accessed_node, list))) {
+-		trans_name = transaction_get_node_name(i, trans, i->node);
+-		if (!trans_name)
+-			/* We are doomed: the transaction is only partial. */
+-			goto err;
+-
+-		set_tdb_key(trans_name, &ta_key);
+-
+-		if (i->modified) {
+-			set_tdb_key(i->node, &key);
+-			if (i->ta_node) {
+-				data = tdb_fetch(tdb_ctx, ta_key);
+-				if (!data.dptr)
+-					goto err;
++		set_tdb_key(i->node, &key);
++		if (i->ta_node) {
++			set_tdb_key(i->trans_name, &ta_key);
++			data = tdb_fetch(tdb_ctx, ta_key);
++			if (data.dptr) {
+ 				hdr = (void *)data.dptr;
+ 				hdr->generation = ++generation;
+-				ret = do_tdb_write(conn, &key, &data, NULL,
+-						   true);
++				*is_corrupt |= do_tdb_write(conn, &key, &data,
++							    NULL, true);
+ 				talloc_free(data.dptr);
++				if (do_tdb_delete(conn, &ta_key, NULL))
++					*is_corrupt = true;
+ 			} else {
+-				/*
+-				 * A node having been created and later deleted
+-				 * in this transaction will have no generation
+-				 * information stored.
+-				 */
+-				ret = (i->generation == NO_GENERATION)
+-				      ? 0 : do_tdb_delete(conn, &key, NULL);
+-			}
+-			if (ret)
+-				goto err;
+-			if (i->fire_watch) {
+-				fire_watches(conn, trans, i->node, NULL,
+-					     i->watch_exact,
+-					     i->perms.p ? &i->perms : NULL);
++				*is_corrupt = true;
+ 			}
++		} else {
++			/*
++			 * A node having been created and later deleted
++			 * in this transaction will have no generation
++			 * information stored.
++			 */
++			*is_corrupt |= (i->generation == NO_GENERATION)
++				       ? false
++				       : do_tdb_delete(conn, &key, NULL);
+ 		}
++		if (i->fire_watch)
++			fire_watches(conn, trans, i->node, NULL, i->watch_exact,
++				     i->perms.p ? &i->perms : NULL);
+ 
+-		if (i->ta_node && do_tdb_delete(conn, &ta_key, NULL))
+-			goto err;
+ 		list_del(&i->list);
+ 		talloc_free(i);
+ 	}
+ 
+ 	return 0;
+-
+-err:
+-	corrupt(conn, "Partial transaction");
+-	return EIO;
+ }
+ 
+ static int destroy_transaction(void *_transaction)
+ {
+ 	struct transaction *trans = _transaction;
+ 	struct accessed_node *i;
+-	char *trans_name;
+ 	TDB_DATA key;
+ 
+ 	wrl_ntransactions--;
+ 	trace_destroy(trans, "transaction");
+ 	while ((i = list_top(&trans->accessed, struct accessed_node, list))) {
+ 		if (i->ta_node) {
+-			trans_name = transaction_get_node_name(i, trans,
+-							       i->node);
+-			if (trans_name) {
+-				set_tdb_key(trans_name, &key);
+-				do_tdb_delete(trans->conn, &key, NULL);
+-			}
++			set_tdb_key(i->trans_name, &key);
++			do_tdb_delete(trans->conn, &key, NULL);
+ 		}
+ 		list_del(&i->list);
+ 		talloc_free(i);
+@@ -556,6 +537,7 @@ int do_transaction_end(const void *ctx, struct connection *conn,
+ {
+ 	const char *arg = onearg(in);
+ 	struct transaction *trans;
++	bool is_corrupt = false;
+ 	int ret;
+ 
+ 	if (!arg || (!streq(arg, "T") && !streq(arg, "F")))
+@@ -579,13 +561,17 @@ int do_transaction_end(const void *ctx, struct connection *conn,
+ 		ret = transaction_fix_domains(trans, false);
+ 		if (ret)
+ 			return ret;
+-		if (finalize_transaction(conn, trans))
+-			return EAGAIN;
++		ret = finalize_transaction(conn, trans, &is_corrupt);
++		if (ret)
++			return ret;
+ 
+ 		wrl_apply_debit_trans_commit(conn);
+ 
+ 		/* fix domain entry for each changed domain */
+ 		transaction_fix_domains(trans, true);
++
++		if (is_corrupt)
++			corrupt(conn, "transaction inconsistency");
+ 	}
+ 	send_ack(conn, XS_TRANSACTION_END);
+ 
+@@ -660,7 +646,7 @@ int check_transactions(struct hashtable *hash)
+ 	struct connection *conn;
+ 	struct transaction *trans;
+ 	struct accessed_node *i;
+-	char *tname, *tnode;
++	char *tname;
+ 
+ 	list_for_each_entry(conn, &connections, list) {
+ 		list_for_each_entry(trans, &conn->transaction_list, list) {
+@@ -672,11 +658,8 @@ int check_transactions(struct hashtable *hash)
+ 			list_for_each_entry(i, &trans->accessed, list) {
+ 				if (!i->ta_node)
+ 					continue;
+-				tnode = transaction_get_node_name(tname, trans,
+-								  i->node);
+-				if (!tnode || !remember_string(hash, tnode))
++				if (!remember_string(hash, i->trans_name))
+ 					goto nomem;
+-				talloc_free(tnode);
+ 			}
+ 
+ 			talloc_free(tname);
+diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h
+index 39d7f81c5127..3417303f9427 100644
+--- a/tools/xenstore/xenstored_transaction.h
++++ b/tools/xenstore/xenstored_transaction.h
+@@ -48,8 +48,8 @@ int __must_check access_node(struct connection *conn, struct node *node,
+ void queue_watches(struct connection *conn, const char *name, bool watch_exact);
+ 
+ /* Prepend the transaction to name if appropriate. */
+-int transaction_prepend(struct connection *conn, const char *name,
+-                        TDB_DATA *key);
++void transaction_prepend(struct connection *conn, const char *name,
++                         TDB_DATA *key);
+ 
+ /* Mark the transaction as failed. This will prevent it to be committed. */
+ void fail_transaction(struct transaction *trans);
+-- 
+2.37.4
+