Commit 4f789786 authored by Henrik Riomar's avatar Henrik Riomar Committed by Milan P. Stanić
Browse files

main/xen: fix XSA-325

This is CVE-2020-29483
parent 1652f88a
......@@ -203,6 +203,7 @@ options="!strip"
# - CVE-2020-29481 XSA-322
# - CVE-2020-29482 XSA-323
# - CVE-2020-29484 XSA-324
# - CVE-2020-29483 XSA-325
case "$CARCH" in
x86*)
......@@ -294,6 +295,8 @@ source="https://downloads.xenproject.org/release/xen/$pkgver/xen-$pkgver.tar.gz
xsa324.patch
xsa325-4.14.patch
xenstored.initd
xenstored.confd
xenconsoled.initd
......@@ -562,6 +565,7 @@ d0ec105a6538bbe6b11ffcbe0620e20f8bfbf4bd14be4f3167d58a7b81e5db7b4be978ba7ec38091
301729f80fd4ea60f2d55a0da3122d97aad11e8fd49e4526e2ddcc37a61fe958a4054c3e6ca3442511f7f1ce33300d1e1b5f53272deb126832de03011c5f57a3 xsa322-4.14-c.patch
a8fd66720a73f8c5af413a47188bc73481ead8c9fcdf2d51accdaa5cbaeb211480d990fa93f0e8376a237031683bd050963a44ccb5737a73bb2a804be489a9a9 xsa323.patch
7cbb488c29d3772d01a40d261c9a1cee6363d99f0182b904153a4ab7a0cb369f8e2d20788acedc45852bbdd8882a74e54f4565775bfe8cae4cb58558823c1d0d xsa324.patch
a757a3ca6673f7604f2f836073604c677668e4871651f6702bbc6506336c0684202b1f985ee9c2577784c1a8c57c663799036938ea8eab2de35a72ed111f3d0f xsa325-4.14.patch
52c43beb2596d645934d0f909f2d21f7587b6898ed5e5e7046799a8ed6d58f7a09c5809e1634fa26152f3fd4f3e7cfa07da7076f01b4a20cc8f5df8b9cb77e50 xenstored.initd
093f7fbd43faf0a16a226486a0776bade5dc1681d281c5946a3191c32d74f9699c6bf5d0ab8de9d1195a2461165d1660788e92a3156c9b3c7054d7b2d52d7ff0 xenstored.confd
3c86ed48fbee0af4051c65c4a3893f131fa66e47bf083caf20c9b6aa4b63fdead8832f84a58d0e27964bc49ec8397251b34e5be5c212c139f556916dc8da9523 xenconsoled.initd
......
From: Harsha Shamsundara Havanur <havanur@amazon.com>
Subject: tools/xenstore: Preserve bad client until they are destroyed
XenStored will kill any connection that it thinks has misbehaved,
this is currently happening in two places:
* In `handle_input()` if the sanity check on the ring and the message
fails.
* In `handle_output()` when failing to write the response in the ring.
As the domain structure is a child of the connection, XenStored will
destroy its view of the domain when killing the connection. This will
result in sending @releaseDomain event to all the watchers.
As the watch event doesn't carry which domain has been released,
the watcher (such as XenStored) will generally go through the list of
domains registers and check if one of them is shutting down/dying.
In the case of a client misbehaving, the domain will likely to be
running, so no action will be performed.
When the domain is effectively destroyed, XenStored will not be aware of
the domain anymore. So the watch event is not going to be sent.
By consequence, the watchers of the event will not release mappings
they may have on the domain. This will result in a zombie domain.
In order to send @releaseDomain event at the correct time, we want
to keep the domain structure until the domain is effectively
shutting-down/dying.
We also want to keep the connection around so we could possibly revive
the connection in the future.
A new flag 'is_ignored' is added to mark whether a connection should be
ignored when checking if there are work to do. Additionally any
transactions, watches, buffers associated to the connection will be
freed as you can't do much with them (restarting the connection will
likely need a reset).
As a side note, when the device model were running in a stubdomain, a
guest would have been able to introduce a use-after-free because there
is two parents for a guest connection.
This is XSA-325.
Reported-by: Pawel Wieczorkiewicz <wipawel@amazon.de>
Signed-off-by: Harsha Shamsundara Havanur <havanur@amazon.com>
Signed-off-by: Julien Grall <jgrall@amazon.com>
Reviewed-by: Juergen Gross <jgross@suse.com>
Reviewed-by: Paul Durrant <paul@xen.org>
diff --git a/tools/xenstore/xenstored_core.c b/tools/xenstore/xenstored_core.c
index af3d17004b3f..27d8f15b6b76 100644
--- a/tools/xenstore/xenstored_core.c
+++ b/tools/xenstore/xenstored_core.c
@@ -1355,6 +1355,32 @@ static struct {
[XS_DIRECTORY_PART] = { "DIRECTORY_PART", send_directory_part },
};
+/*
+ * Keep the connection alive but stop processing any new request or sending
+ * reponse. This is to allow sending @releaseDomain watch event at the correct
+ * moment and/or to allow the connection to restart (not yet implemented).
+ *
+ * All watches, transactions, buffers will be freed.
+ */
+static void ignore_connection(struct connection *conn)
+{
+ struct buffered_data *out, *tmp;
+
+ trace("CONN %p ignored\n", conn);
+
+ conn->is_ignored = true;
+ conn_delete_all_watches(conn);
+ conn_delete_all_transactions(conn);
+
+ list_for_each_entry_safe(out, tmp, &conn->out_list, list) {
+ list_del(&out->list);
+ talloc_free(out);
+ }
+
+ talloc_free(conn->in);
+ conn->in = NULL;
+}
+
static const char *sockmsg_string(enum xsd_sockmsg_type type)
{
if ((unsigned int)type < ARRAY_SIZE(wire_funcs) && wire_funcs[type].str)
@@ -1413,8 +1439,10 @@ static void consider_message(struct connection *conn)
assert(conn->in == NULL);
}
-/* Errors in reading or allocating here mean we get out of sync, so we
- * drop the whole client connection. */
+/*
+ * Errors in reading or allocating here means we get out of sync, so we mark
+ * the connection as ignored.
+ */
static void handle_input(struct connection *conn)
{
int bytes;
@@ -1471,14 +1499,14 @@ static void handle_input(struct connection *conn)
return;
bad_client:
- /* Kill it. */
- talloc_free(conn);
+ ignore_connection(conn);
}
static void handle_output(struct connection *conn)
{
+ /* Ignore the connection if an error occured */
if (!write_messages(conn))
- talloc_free(conn);
+ ignore_connection(conn);
}
struct connection *new_connection(connwritefn_t *write, connreadfn_t *read)
@@ -1494,6 +1522,7 @@ struct connection *new_connection(connwritefn_t *write, connreadfn_t *read)
new->write = write;
new->read = read;
new->can_write = true;
+ new->is_ignored = false;
new->transaction_started = 0;
INIT_LIST_HEAD(&new->out_list);
INIT_LIST_HEAD(&new->watches);
@@ -2186,8 +2215,9 @@ int main(int argc, char *argv[])
if (fds[conn->pollfd_idx].revents
& ~(POLLIN|POLLOUT))
talloc_free(conn);
- else if (fds[conn->pollfd_idx].revents
- & POLLIN)
+ else if ((fds[conn->pollfd_idx].revents
+ & POLLIN) &&
+ !conn->is_ignored)
handle_input(conn);
}
if (talloc_free(conn) == 0)
@@ -2199,8 +2229,9 @@ int main(int argc, char *argv[])
if (fds[conn->pollfd_idx].revents
& ~(POLLIN|POLLOUT))
talloc_free(conn);
- else if (fds[conn->pollfd_idx].revents
- & POLLOUT)
+ else if ((fds[conn->pollfd_idx].revents
+ & POLLOUT) &&
+ !conn->is_ignored)
handle_output(conn);
}
if (talloc_free(conn) == 0)
diff --git a/tools/xenstore/xenstored_core.h b/tools/xenstore/xenstored_core.h
index eb19b71f5f46..196a6fd2b0be 100644
--- a/tools/xenstore/xenstored_core.h
+++ b/tools/xenstore/xenstored_core.h
@@ -80,6 +80,9 @@ struct connection
/* Is this a read-only connection? */
bool can_write;
+ /* Is this connection ignored? */
+ bool is_ignored;
+
/* Buffered incoming data. */
struct buffered_data *in;
diff --git a/tools/xenstore/xenstored_domain.c b/tools/xenstore/xenstored_domain.c
index dc635e9be30c..d5e1e3e9d42d 100644
--- a/tools/xenstore/xenstored_domain.c
+++ b/tools/xenstore/xenstored_domain.c
@@ -286,6 +286,10 @@ bool domain_can_read(struct connection *conn)
if (domain_is_unprivileged(conn) && conn->domain->wrl_credit < 0)
return false;
+
+ if (conn->is_ignored)
+ return false;
+
return (intf->req_cons != intf->req_prod);
}
@@ -303,6 +307,10 @@ bool domain_is_unprivileged(struct connection *conn)
bool domain_can_write(struct connection *conn)
{
struct xenstore_domain_interface *intf = conn->domain->interface;
+
+ if (conn->is_ignored)
+ return false;
+
return ((intf->rsp_prod - intf->rsp_cons) != XENSTORE_RING_SIZE);
}
--
2.17.1
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment