-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2022-42309 / XSA-414
version 2
Xenstore: Guests can crash xenstored
UPDATES IN VERSION 2
====================
Public release.
ISSUE DESCRIPTION
=================
Due to a bug in the fix of XSA-115 a malicious guest can cause xenstored
to use a wrong pointer during node creation in an error path, resulting
in a crash of xenstored or a memory corruption in xenstored causing
further damage.
Entering the error path can be controlled by the guest e.g. by exceeding
the quota value of maximum nodes per domain.
IMPACT
======
A malicious guest can cause xenstored to crash, resulting in the inability
to create new guests or to change the configuration of running guests.
Memory corruption in xenstored or privilege escalation of a guest can't
be ruled out.
VULNERABLE SYSTEMS
==================
All Xen versions with the fix for XSA-115 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 instead of xenstored 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.
xsa414.patch xen-unstable, Xen 4.16.x - 4.15.x
xsa414-4.14.patch Xen 4.14.x - 4.13.x
$ sha256sum xsa414*
aad9be1af22eec504bf45ff651509be9106e7d4ceb7552befcf3152a17e5efbe xsa414.meta
f0683bce3b27dd516367091e845559359c12a193b4e051867b580ea46d58359f xsa414.patch
6eb053052786c738abaf747ea69384fd47525186fa6b6ea247383c7cbfbf3e07 xsa414-4.14.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+58MHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZYVAH/1m7ox0cI4jg17wM8ri+cWi0O4bp68MFQKG887DJ
2WZsObdY3SYkUO1YBMg9qu9l5G11+z3UW8KBznafVPweyt35CZJdq6E82SfNc+uf
6/9hmDvXl3fwNJDP9AQBEKMXHPMjRYmIPaniuQdRgnqKSZNUXefbyHZFuHqKabSq
cIEJebNHyNWYmC5fulu53YHuX2WHCkUhlcYYLfqbqd+THGt6Aqj+1NxS3QZ/7zBC
Jiw1eLjzyOGeARkmobl9FJuQpyB9ZmiyenrJCzFMR3uh0njMnMys95VgWxBH+uBe
ooe2vvcoE9EpY8MPmV3UhA+q3JsIis+dkZ2vJQAjaQAomXQ=
=NNSk
-----END PGP SIGNATURE-----
{
"XSA": 414,
"SupportedVersions": [
"master",
"4.16",
"4.15",
"4.14",
"4.13"
],
"Trees": [
"xen"
],
"Recipes": {
"4.13": {
"Recipes": {
"xen": {
"StableRef": "0be63c2615b268001f7cc9b72ce25eed952737dc",
"Prereqs": [
414
],
"Patches": [
"xsa414-4.14.patch"
]
}
}
},
"4.14": {
"Recipes": {
"xen": {
"StableRef": "016de62747b26ead5a5c763b640fe8e205cd182b",
"Prereqs": [
414
],
"Patches": [
"xsa414-4.14.patch"
]
}
}
},
"4.15": {
"Recipes": {
"xen": {
"StableRef": "816580afdd1730d4f85f64477a242a439af1cdf8",
"Prereqs": [
414
],
"Patches": [
"xsa414.patch"
]
}
}
},
"4.16": {
"Recipes": {
"xen": {
"StableRef": "1bce7fb1f702da4f7a749c6f1457ecb20bf74fca",
"Prereqs": [
412,
414
],
"Patches": [
"xsa414.patch"
]
}
}
},
"master": {
"Recipes": {
"xen": {
"StableRef": "cc4747be8ba157a3b310921e9ee07fb8545aa206",
"Prereqs": [
412,
414
],
"Patches": [
"xsa414.patch"
]
}
}
}
}
}From: Julien Grall <jgrall@amazon.com>
Subject: tools/xenstore: create_node: Don't defer work to undo any changes on
failure
XSA-115 extended destroy_node() to update the node accounting for the
connection. The implementation is assuming the connection is the parent
of the node, however all the nodes are allocated using a separate context
(see process_message()). This will result to crash (or corrupt) xenstored
as the pointer is wrongly used.
In case of an error, any changes to the database or update to the
accounting will now be reverted in create_node() by calling directly
destroy_node(). This has the nice advantage to remove the loop to unset
the destructors in case of success.
Take the opportunity to free the nodes right now as they are not
going to be reachable (the function returns NULL) and are just wasting
resources.
This is XSA-414 / CVE-2022-42309.
Reported-by: Julien Grall <jgrall@amazon.com>
Fixes: 0bfb2101f243 ("tools/xenstore: fix node accounting after failed node creation")
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 8867f93431d4..c30d14cbf2ab 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1084,9 +1084,8 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
return NULL;
}
-static int destroy_node(void *_node)
+static int destroy_node(struct connection *conn, struct node *node)
{
- struct node *node = _node;
TDB_DATA key;
if (streq(node->name, "/"))
@@ -1095,7 +1094,7 @@ static int destroy_node(void *_node)
set_tdb_key(node->name, &key);
tdb_delete(tdb_ctx, key);
- domain_entry_dec(talloc_parent(node), node);
+ domain_entry_dec(conn, node);
return 0;
}
@@ -1104,7 +1103,8 @@ static struct node *create_node(struct connection *conn, const void *ctx,
const char *name,
void *data, unsigned int datalen)
{
- struct node *node, *i;
+ struct node *node, *i, *j;
+ int ret;
node = construct_node(conn, ctx, name);
if (!node)
@@ -1126,23 +1126,40 @@ static struct node *create_node(struct connection *conn, const void *ctx,
/* i->parent is set for each new node, so check quota. */
if (i->parent &&
domain_entry(conn) >= quota_nb_entry_per_domain) {
- errno = ENOSPC;
- return NULL;
+ ret = ENOSPC;
+ goto err;
}
- if (write_node(conn, i, false))
- return NULL;
- /* Account for new node, set destructor for error case. */
- if (i->parent) {
+ ret = write_node(conn, i, false);
+ if (ret)
+ goto err;
+
+ /* Account for new node */
+ if (i->parent)
domain_entry_inc(conn, i);
- talloc_set_destructor(i, destroy_node);
- }
}
- /* OK, now remove destructors so they stay around */
- for (i = node; i->parent; i = i->parent)
- talloc_set_destructor(i, NULL);
return node;
+
+err:
+ /*
+ * We failed to update TDB for some of the nodes. Undo any work that
+ * have already been done.
+ */
+ for (j = node; j != i; j = j->parent)
+ destroy_node(conn, j);
+
+ /* We don't need to keep the nodes around, so free them. */
+ i = node;
+ while (i) {
+ j = i;
+ i = i->parent;
+ talloc_free(j);
+ }
+
+ errno = ret;
+
+ return NULL;
}
/* path, data... */
From: Julien Grall <jgrall@amazon.com>
Subject: tools/xenstore: create_node: Don't defer work to undo any changes on
failure
XSA-115 extended destroy_node() to update the node accounting for the
connection. The implementation is assuming the connection is the parent
of the node, however all the nodes are allocated using a separate context
(see process_message()). This will result to crash (or corrupt) xenstored
as the pointer is wrongly used.
In case of an error, any changes to the database or update to the
accounting will now be reverted in create_node() by calling directly
destroy_node(). This has the nice advantage to remove the loop to unset
the destructors in case of success.
Take the opportunity to free the nodes right now as they are not
going to be reachable (the function returns NULL) and are just wasting
resources.
This is XSA-414 / CVE-2022-42309.
Reported-by: Julien Grall <jgrall@amazon.com>
Fixes: 0bfb2101f243 ("tools/xenstore: fix node accounting after failed node creation")
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 1d05d25a4864..6afe8cb59d7e 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -977,9 +977,8 @@ static struct node *construct_node(struct connection *conn, const void *ctx,
return NULL;
}
-static int destroy_node(void *_node)
+static int destroy_node(struct connection *conn, struct node *node)
{
- struct node *node = _node;
TDB_DATA key;
if (streq(node->name, "/"))
@@ -990,7 +989,7 @@ static int destroy_node(void *_node)
tdb_delete(tdb_ctx, key);
- domain_entry_dec(talloc_parent(node), node);
+ domain_entry_dec(conn, node);
return 0;
}
@@ -999,7 +998,8 @@ static struct node *create_node(struct connection *conn, const void *ctx,
const char *name,
void *data, unsigned int datalen)
{
- struct node *node, *i;
+ struct node *node, *i, *j;
+ int ret;
node = construct_node(conn, ctx, name);
if (!node)
@@ -1021,23 +1021,40 @@ static struct node *create_node(struct connection *conn, const void *ctx,
/* i->parent is set for each new node, so check quota. */
if (i->parent &&
domain_entry(conn) >= quota_nb_entry_per_domain) {
- errno = ENOSPC;
- return NULL;
+ ret = ENOSPC;
+ goto err;
}
- if (write_node(conn, i, false))
- return NULL;
- /* Account for new node, set destructor for error case. */
- if (i->parent) {
+ ret = write_node(conn, i, false);
+ if (ret)
+ goto err;
+
+ /* Account for new node */
+ if (i->parent)
domain_entry_inc(conn, i);
- talloc_set_destructor(i, destroy_node);
- }
}
- /* OK, now remove destructors so they stay around */
- for (i = node; i->parent; i = i->parent)
- talloc_set_destructor(i, NULL);
return node;
+
+err:
+ /*
+ * We failed to update TDB for some of the nodes. Undo any work that
+ * have already been done.
+ */
+ for (j = node; j != i; j = j->parent)
+ destroy_node(conn, j);
+
+ /* We don't need to keep the nodes around, so free them. */
+ i = node;
+ while (i) {
+ j = i;
+ i = i->parent;
+ talloc_free(j);
+ }
+
+ errno = ret;
+
+ return NULL;
}
/* path, data... */
© 2016 - 2024 Red Hat, Inc.