File xsa440.patch of Package xen.33138

Overview Repositories Revisions Requests Users Attributes Meta

File xsa440.patch of Package xen.33138

From 05e901796cd70c112307860df1fc39804bae00a5 Mon Sep 17 00:00:00 2001
From: Julien Grall <jgrall@amazon.com>
Date: Fri, 22 Sep 2023 11:32:16 +0100
Subject: [PATCH] tools/xenstored: domain_entry_fix(): Handle conflicting
 transaction

The function domain_entry_fix() will be initially called to check if the
quota is correct before attempt to commit any nodes. So it would be
possible that accounting is temporarily negative. This is the case
in the following sequence:

1) Create 50 nodes
  2) Start two transactions
  3) Delete all the nodes in each transaction
  4) Commit the two transactions

Because the first transaction will have succeed and updated the
accounting, there is no guarantee that 'd->nbentry + num' will still
be above 0. So the assert() would be triggered.
The assert() was introduced in dbef1f748289 ("tools/xenstore: simplify
and fix per domain node accounting") with the assumption that the
value can't be negative. As this is not true revert to the original
check but restricted to the path where we don't update. Take the
opportunity to explain the rationale behind the check.

This CVE-2023-34323 / XSA-440.

Fixes: dbef1f748289 ("tools/xenstore: simplify and fix per domain node accounting")
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>

---
 tools/xenstore/xenstored_domain.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -1030,10 +1030,21 @@ int domain_entry_fix(unsigned int domid,
 	}
 
 	cnt = d->nbentry + num;
-	assert(cnt >= 0);
 
 	if (update)
+	{
+		assert(cnt >= 0);
 		d->nbentry = cnt;
+	} else if (cnt < 0) {
+		/*
+		 * In a transaction when a node is being added/removed AND
+		 * the same node has been added/removed outside the
+		 * transaction in parallel, the result value may be negative.
+		 * This is no problem, as the transaction will fail due to
+		 * the resulting conflict. So override 'cnt'.
+		 */
+		cnt = 0;
+	}
 
 	return domid_is_unprivileged(domid) ? cnt : 0;
 }

Places

File xsa440.patch of Package xen.33138

Places