[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] [xen stable-4.15] tools/xenstored: domain_entry_fix(): Handle conflicting transaction
commit 0a70ce96deebbe3074708b618bab11cd802491c8 Author: Julien Grall <jgrall@xxxxxxxxxx> AuthorDate: Fri Sep 22 11:32:16 2023 +0100 Commit: Andrew Cooper <andrew.cooper3@xxxxxxxxxx> CommitDate: Wed Sep 27 16:30:18 2023 +0100 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@xxxxxxxxxx> Reviewed-by: Juergen Gross <jgross@xxxxxxxx> (cherry picked from commit c4e05c97f57d236040d1da5c1fbf6e3699dc86ea) --- tools/xenstore/xenstored_domain.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c index 8cc36ee44c..697a7cfe64 100644 --- a/tools/xenstore/xenstored_domain.c +++ b/tools/xenstore/xenstored_domain.c @@ -1068,10 +1068,20 @@ int domain_entry_fix(unsigned int domid, int num, bool update) } cnt = d->nbentry + num; - assert(cnt >= 0); - if (update) + 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; } -- generated by git-patchbot for /home/xen/git/xen.git#stable-4.15
|
![]() |
Lists.xenproject.org is hosted with RackSpace, monitoring our |