-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2022-42310 / XSA-415
version 2
Xenstore: Guests can create orphaned Xenstore nodes
UPDATES IN VERSION 2
====================
Public release.
ISSUE DESCRIPTION
=================
By creating multiple nodes inside a transaction resulting in an error,
a malicious guest can create orphaned nodes in the Xenstore data base,
as the cleanup after the error will not remove all nodes already
created. When the transaction is committed after this situation, nodes
without a valid parent can be made permanent in the data base.
IMPACT
======
A malicious guest can cause inconsistencies in the xenstored data base,
resulting in unusual error responses or memory leaks in xenstored. This
can finally cause Denial of Service situations or long running error
recoveries of xenstored.
VULNERABLE SYSTEMS
==================
Systems with Xen version 4.9 and newer running the C variant of Xenstore
(xenstored or xenstore-stubdom) are vulnerable.
Systems using the Ocaml variant of Xenstore (oxenstored) are not vulnerable.
MITIGATION
==========
Using oxenstored will avoid the vulnerability.
CREDITS
=======
This issue was discovered by Julien Grall of Amazon.
RESOLUTION
==========
Applying the appropriate attached patch resolves this issue.
Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball. Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.
xsa415.patch xen-unstable, Xen 4.16.x
xsa415-4.15.patch Xen 4.15.x
xsa415-4.14.patch Xen 4.14.x - 4.13.x
$ sha256sum xsa415*
ff973fd3d0af2b45ba46ba74410204a60fcba30b0d0830c591dc827eac9ae484 xsa415.meta
bc5b33bbef18c0fb15d6da6760ece9ef7f6f2cfab78664aee533ff717b379e3b xsa415.patch
243e7e35ba94973252a6381977af2cf70774abfd0bfd5d0015179b94c832453e xsa415-4.14.patch
7b18b510b811551025cd2a86d654ee776b5003172ab468e7e86a0c6d892f4629 xsa415-4.15.patch
$
DEPLOYMENT DURING EMBARGO
=========================
Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.
But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).
Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.
(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable. This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)
For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmNg+6IMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZm88H/inrzV4zw8Po/g59rq1hUrCE/L4KwAemf5ZmWMK8
Unka74TyN2j47wous4EbBstzQQtOvf7GP2OT68qpIlqaZSAGcu+7x6TPx3M8q8kM
ZFzqcDYvNye8KrUCNp9pVJIV2Y8b3JLAZXCvxxGK++yECGMjTh5ZkxzdiNK/t9NO
+TmhH7CHFzkiO25Ch/8+vlwMs6eH/rKFLUVbEU/ZiD9L/P84xQr1EORhAhDJorx1
SLyprG0BlaCUIA/YbQVEftqHiG0J6ikuBYJGBHyQGVEV/MqSXGCUB/Eee6nzH4fH
1USXmeQ27OMsKwOJXyxFvrCgmKdeTNDcx0KSzSPFrED9rSc=
=hu/k
-----END PGP SIGNATURE-----
{
"XSA": 415,
"SupportedVersions": [
"master",
"4.16",
"4.15",
"4.14",
"4.13"
],
"Trees": [
"xen"
],
"Recipes": {
"4.13": {
"Recipes": {
"xen": {
"StableRef": "0be63c2615b268001f7cc9b72ce25eed952737dc",
"Prereqs": [
414
],
"Patches": [
"xsa415-4.14.patch"
]
}
}
},
"4.14": {
"Recipes": {
"xen": {
"StableRef": "016de62747b26ead5a5c763b640fe8e205cd182b",
"Prereqs": [
414
],
"Patches": [
"xsa415-4.14.patch"
]
}
}
},
"4.15": {
"Recipes": {
"xen": {
"StableRef": "816580afdd1730d4f85f64477a242a439af1cdf8",
"Prereqs": [
414
],
"Patches": [
"xsa415-4.15.patch"
]
}
}
},
"4.16": {
"Recipes": {
"xen": {
"StableRef": "1bce7fb1f702da4f7a749c6f1457ecb20bf74fca",
"Prereqs": [
412,
414
],
"Patches": [
"xsa415.patch"
]
}
}
},
"master": {
"Recipes": {
"xen": {
"StableRef": "cc4747be8ba157a3b310921e9ee07fb8545aa206",
"Prereqs": [
412,
414
],
"Patches": [
"xsa415.patch"
]
}
}
}
}
}From: Julien Grall <jgrall@amazon.com>
Subject: tools/xenstore: Fail a transaction if it is not possible to create a
node
Commit f2bebf72c4d5 "xenstore: rework of transaction handling" moved
out from copying the entire database everytime a new transaction is
opened to track the list of nodes changed.
The content of all the nodes accessed during a transaction will be
temporarily stored in TDB using a different key.
The function create_node() may write/update multiple nodes if the child
doesn't exist. In case of a failure, the function will revert any
changes (this include any update to TDB). Unfortunately, the function
which reverts the changes (i.e. destroy_node()) will not use the correct
key to delete any update or even request the transaction to fail.
This means that if a client decide to go ahead with committing the
transaction, orphan nodes will be created because they were not linked
to an existing node (create_node() will write the nodes backwards).
Once some nodes have been partially updated in a transaction, it is not
easily possible to undo any changes. So rather than continuing and hit
weird issue while committing, it is much saner to fail the transaction.
This will have an impact on any client that decides to commit even if it
can't write a node. Although, it is not clear why a normal client would
want to do that...
Lastly, update destroy_node() to use the correct key for deleting the
node. Rather than recreating it (this will allocate memory and
therefore fail), stash the key in the structure node.
This is XSA-415 / CVE-2022-42310.
Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index c30d14cbf2ab..55b79e4c032e 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -562,15 +562,17 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node,
return 0;
}
+/*
+ * Write the node. If the node is written, caller can find the key used in
+ * node->key. This can later be used if the change needs to be reverted.
+ */
static int write_node(struct connection *conn, struct node *node,
bool no_quota_check)
{
- TDB_DATA key;
-
- if (access_node(conn, node, NODE_ACCESS_WRITE, &key))
+ if (access_node(conn, node, NODE_ACCESS_WRITE, &node->key))
return errno;
- return write_node_raw(conn, &key, node, no_quota_check);
+ return write_node_raw(conn, &node->key, node, no_quota_check);
}
unsigned int perm_for_conn(struct connection *conn,
@@ -1086,16 +1088,21 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
static int destroy_node(struct connection *conn, struct node *node)
{
- TDB_DATA key;
-
if (streq(node->name, "/"))
corrupt(NULL, "Destroying root node!");
- set_tdb_key(node->name, &key);
- tdb_delete(tdb_ctx, key);
+ tdb_delete(tdb_ctx, node->key);
domain_entry_dec(conn, node);
+ /*
+ * It is not possible to easily revert the changes in a transaction.
+ * So if the failure happens in a transaction, mark it as fail to
+ * prevent any commit.
+ */
+ if ( conn->transaction )
+ fail_transaction(conn->transaction);
+
return 0;
}
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 742812a97469..7d0fe77e7989 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -155,6 +155,8 @@ struct node_perms {
struct node {
const char *name;
+ /* Key used to update TDB */
+ TDB_DATA key;
/* Parent (optional) */
struct node *parent;
diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
index cd07fb0f218b..faf6c930e42a 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -580,6 +580,11 @@ void transaction_entry_dec(struct transaction *trans, unsigned int domid)
list_add_tail(&d->list, &trans->changed_domains);
}
+void fail_transaction(struct transaction *trans)
+{
+ trans->fail = true;
+}
+
void conn_delete_all_transactions(struct connection *conn)
{
struct transaction *trans;
diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h
index 43a162bea3f3..14062730e3c9 100644
--- a/tools/xenstore/xenstored_transaction.h
+++ b/tools/xenstore/xenstored_transaction.h
@@ -46,6 +46,9 @@ int access_node(struct connection *conn, struct node *node,
int 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);
+
void conn_delete_all_transactions(struct connection *conn);
int check_transactions(struct hashtable *hash);
From: Julien Grall <jgrall@amazon.com>
Subject: tools/xenstore: Fail a transaction if it is not possible to create a
node
Commit f2bebf72c4d5 "xenstore: rework of transaction handling" moved
out from copying the entire database everytime a new transaction is
opened to track the list of nodes changed.
The content of all the nodes accessed during a transaction will be
temporarily stored in TDB using a different key.
The function create_node() may write/update multiple nodes if the child
doesn't exist. In case of a failure, the function will revert any
changes (this include any update to TDB). Unfortunately, the function
which reverts the changes (i.e. destroy_node()) will not use the correct
key to delete any update or even request the transaction to fail.
This means that if a client decide to go ahead with committing the
transaction, orphan nodes will be created because they were not linked
to an existing node (create_node() will write the nodes backwards).
Once some nodes have been partially updated in a transaction, it is not
easily possible to undo any changes. So rather than continuing and hit
weird issue while committing, it is much saner to fail the transaction.
This will have an impact on any client that decides to commit even if it
can't write a node. Although, it is not clear why a normal client would
want to do that...
Lastly, update destroy_node() to use the correct key for deleting the
node. Rather than recreating it (this will allocate memory and
therefore fail), stash the key in the structure node.
This is XSA-415 / CVE-2022-42310.
Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index 6afe8cb59d7e..8e91b554984d 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -468,15 +468,17 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node,
return 0;
}
+/*
+ * Write the node. If the node is written, caller can find the key used in
+ * node->key. This can later be used if the change needs to be reverted.
+ */
static int write_node(struct connection *conn, struct node *node,
bool no_quota_check)
{
- TDB_DATA key;
-
- if (access_node(conn, node, NODE_ACCESS_WRITE, &key))
+ if (access_node(conn, node, NODE_ACCESS_WRITE, &node->key))
return errno;
- return write_node_raw(conn, &key, node, no_quota_check);
+ return write_node_raw(conn, &node->key, node, no_quota_check);
}
enum xs_perm_type perm_for_conn(struct connection *conn,
@@ -979,18 +981,21 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
static int destroy_node(struct connection *conn, struct node *node)
{
- TDB_DATA key;
-
if (streq(node->name, "/"))
corrupt(NULL, "Destroying root node!");
- key.dptr = (void *)node->name;
- key.dsize = strlen(node->name);
-
- tdb_delete(tdb_ctx, key);
+ tdb_delete(tdb_ctx, node->key);
domain_entry_dec(conn, node);
+ /*
+ * It is not possible to easily revert the changes in a transaction.
+ * So if the failure happens in a transaction, mark it as fail to
+ * prevent any commit.
+ */
+ if ( conn->transaction )
+ fail_transaction(conn->transaction);
+
return 0;
}
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 196a6fd2b0be..9369c4cbfd26 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -119,6 +119,8 @@ struct node_perms {
struct node {
const char *name;
+ /* Key used to update TDB */
+ TDB_DATA key;
/* Parent (optional) */
struct node *parent;
diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
index 2881f3b2e44d..4ffa18311120 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -582,6 +582,11 @@ void transaction_entry_dec(struct transaction *trans, unsigned int domid)
list_add_tail(&d->list, &trans->changed_domains);
}
+void fail_transaction(struct transaction *trans)
+{
+ trans->fail = true;
+}
+
void conn_delete_all_transactions(struct connection *conn)
{
struct transaction *trans;
diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h
index 43a162bea3f3..14062730e3c9 100644
--- a/tools/xenstore/xenstored_transaction.h
+++ b/tools/xenstore/xenstored_transaction.h
@@ -46,6 +46,9 @@ int access_node(struct connection *conn, struct node *node,
int 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);
+
void conn_delete_all_transactions(struct connection *conn);
int check_transactions(struct hashtable *hash);
From: Julien Grall <jgrall@amazon.com>
Subject: tools/xenstore: Fail a transaction if it is not possible to create a
node
Commit f2bebf72c4d5 "xenstore: rework of transaction handling" moved
out from copying the entire database everytime a new transaction is
opened to track the list of nodes changed.
The content of all the nodes accessed during a transaction will be
temporarily stored in TDB using a different key.
The function create_node() may write/update multiple nodes if the child
doesn't exist. In case of a failure, the function will revert any
changes (this include any update to TDB). Unfortunately, the function
which reverts the changes (i.e. destroy_node()) will not use the correct
key to delete any update or even request the transaction to fail.
This means that if a client decide to go ahead with committing the
transaction, orphan nodes will be created because they were not linked
to an existing node (create_node() will write the nodes backwards).
Once some nodes have been partially updated in a transaction, it is not
easily possible to undo any changes. So rather than continuing and hit
weird issue while committing, it is much saner to fail the transaction.
This will have an impact on any client that decides to commit even if it
can't write a node. Although, it is not clear why a normal client would
want to do that...
Lastly, update destroy_node() to use the correct key for deleting the
node. Rather than recreating it (this will allocate memory and
therefore fail), stash the key in the structure node.
This is XSA-415 / CVE-2022-42310.
Reported-by: Julien Grall <jgrall@amazon.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index a00c49e404a1..b28c2c66b53b 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -531,15 +531,17 @@ int write_node_raw(struct connection *conn, TDB_DATA *key, struct node *node,
return 0;
}
+/*
+ * Write the node. If the node is written, caller can find the key used in
+ * node->key. This can later be used if the change needs to be reverted.
+ */
static int write_node(struct connection *conn, struct node *node,
bool no_quota_check)
{
- TDB_DATA key;
-
- if (access_node(conn, node, NODE_ACCESS_WRITE, &key))
+ if (access_node(conn, node, NODE_ACCESS_WRITE, &node->key))
return errno;
- return write_node_raw(conn, &key, node, no_quota_check);
+ return write_node_raw(conn, &node->key, node, no_quota_check);
}
enum xs_perm_type perm_for_conn(struct connection *conn,
@@ -1056,16 +1058,21 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
static int destroy_node(struct connection *conn, struct node *node)
{
- TDB_DATA key;
-
if (streq(node->name, "/"))
corrupt(NULL, "Destroying root node!");
- set_tdb_key(node->name, &key);
- tdb_delete(tdb_ctx, key);
+ tdb_delete(tdb_ctx, node->key);
domain_entry_dec(conn, node);
+ /*
+ * It is not possible to easily revert the changes in a transaction.
+ * So if the failure happens in a transaction, mark it as fail to
+ * prevent any commit.
+ */
+ if ( conn->transaction )
+ fail_transaction(conn->transaction);
+
return 0;
}
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index 0c9a0961b57e..900336afa426 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -148,6 +148,8 @@ struct node_perms {
struct node {
const char *name;
+ /* Key used to update TDB */
+ TDB_DATA key;
/* Parent (optional) */
struct node *parent;
diff --git a/tools/xenstore/xenstored_transaction.c b/tools/xenstore/xenstored_transaction.c
index cd07fb0f218b..faf6c930e42a 100644
--- a/tools/xenstore/xenstored_transaction.c
+++ b/tools/xenstore/xenstored_transaction.c
@@ -580,6 +580,11 @@ void transaction_entry_dec(struct transaction *trans, unsigned int domid)
list_add_tail(&d->list, &trans->changed_domains);
}
+void fail_transaction(struct transaction *trans)
+{
+ trans->fail = true;
+}
+
void conn_delete_all_transactions(struct connection *conn)
{
struct transaction *trans;
diff --git a/tools/xenstore/xenstored_transaction.h b/tools/xenstore/xenstored_transaction.h
index 43a162bea3f3..14062730e3c9 100644
--- a/tools/xenstore/xenstored_transaction.h
+++ b/tools/xenstore/xenstored_transaction.h
@@ -46,6 +46,9 @@ int access_node(struct connection *conn, struct node *node,
int 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);
+
void conn_delete_all_transactions(struct connection *conn);
int check_transactions(struct hashtable *hash);
© 2016 - 2024 Red Hat, Inc.