* Xen Security Advisory 368 v3 (CVE-2021-28687) - HVM soft-reset crashes toolstack
@ 2021-03-18 13:56 Xen.org security team
0 siblings, 0 replies; only message in thread
From: Xen.org security team @ 2021-03-18 13:56 UTC (permalink / raw)
To: xen-announce, xen-devel, xen-users, oss-security; +Cc: Xen.org security team
[-- Attachment #1: Type: text/plain, Size: 4969 bytes --]
-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2021-28687 / XSA-368
version 3
HVM soft-reset crashes toolstack
UPDATES IN VERSION 3
====================
CVE assigned.
ISSUE DESCRIPTION
=================
libxl requires all data structures passed across its public interface
to be initialized before use and disposed of afterwards by calling a
specific set of functions. Many internal data structures also require
this initialize / dispose discipline, but not all of them.
When the "soft reset" feature was implemented, the
libxl__domain_suspend_state structure didn't require any
initialization or disposal. At some point later, an initialization
function was introduced for the structure; but the "soft reset" path
wasn't refactored to call the initialization function. When a guest
nwo initiates a "soft reboot", uninitialized data structure leads to
an assert() when later code finds the structure in an unexpected
state.
The effect of this is to crash the process monitoring the guest. How
this affects the system depends on the structure of the toolstack.
For xl, this will have no security-relevant effect: every VM has its
own independent monitoring process, which contains no state. The
domain in question will hang in a crashed state, but can be destroyed
by `xl destroy` just like any other non-cooperating domain.
For daemon-based toolstacks linked against libxl, such as libvirt,
this will crash the toolstack, losing the state of any in-progress
operations (localized DoS), and preventing further administrator
operations unless the daemon is configured to restart automatically
(system-wide DoS). If crashes "leak" resources, then repeated crashes
could use up resources, also causing a system-wide DoS.
IMPACT
======
A malicious guest can crash the management daemon, leading to at least
a localized, possibly system-wide denial-of-service.
VULNERABLE SYSTEMS
==================
Only Xen versions 4.12 through 4.14 are affected. Earlier versions
are not affected.
The issue affects only systems with a guest monitoring process, which
is linked against libxl, and which is important other than simply for
the functioning of one particular guest. libvirt is one common
toolstack affected. Systems using the `xl` command-line tool should
generally suffer no security-relevant effects.
The xapi toolstack does not currently link against libxl, and so is
not affected.
MITIGATION
==========
Ensuring that any management daemons are restarted automatically after
a crash will partially mitigate the issue.
CREDITS
=======
This issue was discovered by Olaf Hering.
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.
xsa368.patch xen-unstable
xsa368-4.14.patch Xen 4.14.x
xsa368-4.13.patch Xen 4.13.x - Xen 4.12.x
$ sha256sum xsa368*
e80f33c3ce45372fef7bd91ec71b2b66e557176b79f9771872ce111bfff34150 xsa368.meta
b82f2b110514cdf47a2688913ad5af68b01050751d56705a15ddf9a970b6fa0d xsa368.patch
636df70ae5eaf00b50ef0b5ac219a2aeda771c66833fae88e7ee43b18ae889f4 xsa368-4.13.patch
55bbe59c75b69f493e364dfcf6cdbc7db4acd32dbf0b4d2466815b7c1f1823ce xsa368-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/4UyVfoK9kFAmBTXAAMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZdgcH/RTW41tLPh8KHJ+82qefaI2EUBK3nmNnR5hnye3c
9GPP/QB7QdHp+JSIRTAZxOayBQeFEcYSX/5VxDypIiqT02wHS9hDr3jcpOfGLcdt
MiN9kB3vYqe353Lask0mN7AX3J5v3wvrYzBRx9ccaYcX/Jcubrx6Jy5laQSYpTUu
4GCeLZQ2tHI8N3ZHiKI7YUyxmn9vKgvFil1gyuk8L5x6npnW4ixdWF0MRyHe7wbS
dbZbug0g6bbJbs4CFZbm1CbQjGGOwznfT8z9ppmgPdi+33X+Cimz3wlbpXeJKpZk
/nJObobdPGk7ClChvUjntv0oaZ+2zFoUoe3Yc08aa+B29e8=
=Dehk
-----END PGP SIGNATURE-----
[-- Attachment #2: xsa368.meta --]
[-- Type: application/octet-stream, Size: 1075 bytes --]
{
"XSA": 368,
"SupportedVersions": [
"master",
"4.14",
"4.13",
"4.12"
],
"Trees": [
"xen"
],
"Recipes": {
"4.12": {
"Recipes": {
"xen": {
"StableRef": "4cf5929606adc2fb1ab4e2921c14ba4b8046ecd1",
"Prereqs": [],
"Patches": [
"xsa368-4.13.patch"
]
}
}
},
"4.13": {
"Recipes": {
"xen": {
"StableRef": "d7a1e06efd3ae2b16d5bb335932376b7d7eaf633",
"Prereqs": [],
"Patches": [
"xsa368-4.13.patch"
]
}
}
},
"4.14": {
"Recipes": {
"xen": {
"StableRef": "b0b734a8b3e516ff1040884b755a8d47afed31ea",
"Prereqs": [],
"Patches": [
"xsa368-4.14.patch"
]
}
}
},
"master": {
"Recipes": {
"xen": {
"StableRef": "4834936549f788378918da8e9bc97df7dd3ee16d",
"Prereqs": [],
"Patches": [
"xsa368.patch"
]
}
}
}
}
}
[-- Attachment #3: xsa368.patch --]
[-- Type: application/octet-stream, Size: 4605 bytes --]
From eab2b4ab06419b82be1e2cfcdc5ba2a462528d68 Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Wed, 24 Feb 2021 18:39:20 +0000
Subject: [PATCH] libxl: Fix domain soft reset state handling
In do_domain_soft_reset(), a `libxl__domain_suspend_state' is used
without been properly initialised and disposed of. This lead do a
abort() in libxl due to the `dsps.qmp' state been used before been
initialised:
libxl__ev_qmp_send: Assertion `ev->state == qmp_state_disconnected || ev->state == qmp_state_connected' failed.
Once initialised, `dsps' also needs to be disposed of as the `qmp'
state might still be in the `Connected' state in the callback for
libxl__domain_suspend_device_model(). So this patch adds
libxl__domain_suspend_dispose() which can be called from the two
places where we need to dispose of `dsps'.
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Tested-by: Olaf Hering <olaf@aepfle.de>
---
tools/libs/light/libxl_create.c | 11 ++++++++---
tools/libs/light/libxl_dom_suspend.c | 15 +++++++++++----
tools/libs/light/libxl_internal.h | 2 ++
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/tools/libs/light/libxl_create.c b/tools/libs/light/libxl_create.c
index 46f68da697..dca2766805 100644
--- a/tools/libs/light/libxl_create.c
+++ b/tools/libs/light/libxl_create.c
@@ -2179,9 +2179,7 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
state->console_tty = libxl__strdup(gc, console_tty);
dss->ao = ao;
- dss->domid = dss->dsps.domid = domid;
- dss->dsps.dm_savefile = GCSPRINTF(LIBXL_DEVICE_MODEL_SAVE_FILE".%d",
- domid);
+ dss->domid = domid;
rc = libxl__save_emulator_xenstore_data(dss, &srs->toolstack_buf,
&srs->toolstack_len);
@@ -2191,6 +2189,11 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
}
dss->dsps.ao = ao;
+ dss->dsps.domid = domid;
+ dss->dsps.live = false;
+ rc = libxl__domain_suspend_init(egc, &dss->dsps, d_config->b_info.type);
+ if (rc)
+ goto out;
dss->dsps.callback_device_model_done = soft_reset_dm_suspended;
libxl__domain_suspend_device_model(egc, &dss->dsps); /* must be last */
@@ -2209,6 +2212,8 @@ static void soft_reset_dm_suspended(libxl__egc *egc,
CONTAINER_OF(dsps, *srs, dss.dsps);
libxl__app_domain_create_state *cdcs = &srs->cdcs;
+ libxl__domain_suspend_dispose(gc, dsps);
+
/*
* Ask all backends to disconnect by removing the domain from
* xenstore. On the creation path the domain will be introduced to
diff --git a/tools/libs/light/libxl_dom_suspend.c b/tools/libs/light/libxl_dom_suspend.c
index f7823bbc8f..4fa22bb739 100644
--- a/tools/libs/light/libxl_dom_suspend.c
+++ b/tools/libs/light/libxl_dom_suspend.c
@@ -67,6 +67,16 @@ out:
return rc;
}
+void libxl__domain_suspend_dispose(libxl__gc *gc,
+ libxl__domain_suspend_state *dsps)
+{
+ libxl__xswait_stop(gc, &dsps->pvcontrol);
+ libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
+ libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
+ libxl__ev_time_deregister(gc, &dsps->guest_timeout);
+ libxl__ev_qmp_dispose(gc, &dsps->qmp);
+}
+
/*----- callbacks, called by xc_domain_save -----*/
void libxl__domain_suspend_device_model(libxl__egc *egc,
@@ -388,10 +398,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
{
EGC_GC;
assert(!libxl__xswait_inuse(&dsps->pvcontrol));
- libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
- libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
- libxl__ev_time_deregister(gc, &dsps->guest_timeout);
- libxl__ev_qmp_dispose(gc, &dsps->qmp);
+ libxl__domain_suspend_dispose(gc, dsps);
dsps->callback_common_done(egc, dsps, rc);
}
diff --git a/tools/libs/light/libxl_internal.h b/tools/libs/light/libxl_internal.h
index 028bc013d9..c6a4a187f5 100644
--- a/tools/libs/light/libxl_internal.h
+++ b/tools/libs/light/libxl_internal.h
@@ -3617,6 +3617,8 @@ struct libxl__domain_suspend_state {
int libxl__domain_suspend_init(libxl__egc *egc,
libxl__domain_suspend_state *dsps,
libxl_domain_type type);
+void libxl__domain_suspend_dispose(libxl__gc *gc,
+ libxl__domain_suspend_state *dsps);
/* calls dsps->callback_device_model_done when done
* may synchronously calls this callback */
--
2.30.1
[-- Attachment #4: xsa368-4.13.patch --]
[-- Type: application/octet-stream, Size: 4574 bytes --]
From a733fcca97d4e0d7503198ba1dd739a5d7a00dac Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Wed, 24 Feb 2021 18:39:20 +0000
Subject: [PATCH] libxl: Fix domain soft reset state handling
In do_domain_soft_reset(), a `libxl__domain_suspend_state' is used
without been properly initialised and disposed of. This lead do a
abort() in libxl due to the `dsps.qmp' state been used before been
initialised:
libxl__ev_qmp_send: Assertion `ev->state == qmp_state_disconnected || ev->state == qmp_state_connected' failed.
Once initialised, `dsps' also needs to be disposed of as the `qmp'
state might still be in the `Connected' state in the callback for
libxl__domain_suspend_device_model(). So this patch adds
libxl__domain_suspend_dispose() which can be called from the two
places where we need to dispose of `dsps'.
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Tested-by: Olaf Hering <olaf@aepfle.de>
---
tools/libxl/libxl_create.c | 11 ++++++++---
tools/libxl/libxl_dom_suspend.c | 15 +++++++++++----
tools/libxl/libxl_internal.h | 2 ++
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 32d45dcef0..651ad18d2d 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1974,9 +1974,7 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
state->console_tty = libxl__strdup(gc, console_tty);
dss->ao = ao;
- dss->domid = dss->dsps.domid = domid_soft_reset;
- dss->dsps.dm_savefile = GCSPRINTF(LIBXL_DEVICE_MODEL_SAVE_FILE".%d",
- domid_soft_reset);
+ dss->domid = domid_soft_reset;
rc = libxl__save_emulator_xenstore_data(dss, &srs->toolstack_buf,
&srs->toolstack_len);
@@ -1986,6 +1984,11 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
}
dss->dsps.ao = ao;
+ dss->dsps.domid = domid_soft_reset;
+ dss->dsps.live = false;
+ rc = libxl__domain_suspend_init(egc, &dss->dsps, d_config->b_info.type);
+ if (rc)
+ goto out;
dss->dsps.callback_device_model_done = soft_reset_dm_suspended;
libxl__domain_suspend_device_model(egc, &dss->dsps); /* must be last */
@@ -2004,6 +2007,8 @@ static void soft_reset_dm_suspended(libxl__egc *egc,
CONTAINER_OF(dsps, *srs, dss.dsps);
libxl__app_domain_create_state *cdcs = &srs->cdcs;
+ libxl__domain_suspend_dispose(gc, dsps);
+
/*
* Ask all backends to disconnect by removing the domain from
* xenstore. On the creation path the domain will be introduced to
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 25d1571895..2a280f69a1 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -67,6 +67,16 @@ out:
return rc;
}
+void libxl__domain_suspend_dispose(libxl__gc *gc,
+ libxl__domain_suspend_state *dsps)
+{
+ libxl__xswait_stop(gc, &dsps->pvcontrol);
+ libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
+ libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
+ libxl__ev_time_deregister(gc, &dsps->guest_timeout);
+ libxl__ev_qmp_dispose(gc, &dsps->qmp);
+}
+
/*----- callbacks, called by xc_domain_save -----*/
void libxl__domain_suspend_device_model(libxl__egc *egc,
@@ -388,10 +398,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
{
EGC_GC;
assert(!libxl__xswait_inuse(&dsps->pvcontrol));
- libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
- libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
- libxl__ev_time_deregister(gc, &dsps->guest_timeout);
- libxl__ev_qmp_dispose(gc, &dsps->qmp);
+ libxl__domain_suspend_dispose(gc, dsps);
dsps->callback_common_done(egc, dsps, rc);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 247518a7ac..5b4795908b 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3569,6 +3569,8 @@ struct libxl__domain_suspend_state {
int libxl__domain_suspend_init(libxl__egc *egc,
libxl__domain_suspend_state *dsps,
libxl_domain_type type);
+void libxl__domain_suspend_dispose(libxl__gc *gc,
+ libxl__domain_suspend_state *dsps);
/* calls dsps->callback_device_model_done when done
* may synchronously calls this callback */
--
2.30.1
[-- Attachment #5: xsa368-4.14.patch --]
[-- Type: application/octet-stream, Size: 4530 bytes --]
From b1d5e033df1858edd6fa328abd126522947440aa Mon Sep 17 00:00:00 2001
From: Anthony PERARD <anthony.perard@citrix.com>
Date: Wed, 24 Feb 2021 18:39:20 +0000
Subject: [PATCH] libxl: Fix domain soft reset state handling
In do_domain_soft_reset(), a `libxl__domain_suspend_state' is used
without been properly initialised and disposed of. This lead do a
abort() in libxl due to the `dsps.qmp' state been used before been
initialised:
libxl__ev_qmp_send: Assertion `ev->state == qmp_state_disconnected || ev->state == qmp_state_connected' failed.
Once initialised, `dsps' also needs to be disposed of as the `qmp'
state might still be in the `Connected' state in the callback for
libxl__domain_suspend_device_model(). So this patch adds
libxl__domain_suspend_dispose() which can be called from the two
places where we need to dispose of `dsps'.
Reported-by: Olaf Hering <olaf@aepfle.de>
Signed-off-by: Anthony PERARD <anthony.perard@citrix.com>
Reviewed-by: Ian Jackson <iwj@xenproject.org>
Tested-by: Olaf Hering <olaf@aepfle.de>
---
tools/libxl/libxl_create.c | 11 ++++++++---
tools/libxl/libxl_dom_suspend.c | 15 +++++++++++----
tools/libxl/libxl_internal.h | 2 ++
3 files changed, 21 insertions(+), 7 deletions(-)
diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index 2814818e34..83b0eb00bf 100644
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -2174,9 +2174,7 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
state->console_tty = libxl__strdup(gc, console_tty);
dss->ao = ao;
- dss->domid = dss->dsps.domid = domid;
- dss->dsps.dm_savefile = GCSPRINTF(LIBXL_DEVICE_MODEL_SAVE_FILE".%d",
- domid);
+ dss->domid = domid;
rc = libxl__save_emulator_xenstore_data(dss, &srs->toolstack_buf,
&srs->toolstack_len);
@@ -2186,6 +2184,11 @@ static int do_domain_soft_reset(libxl_ctx *ctx,
}
dss->dsps.ao = ao;
+ dss->dsps.domid = domid;
+ dss->dsps.live = false;
+ rc = libxl__domain_suspend_init(egc, &dss->dsps, d_config->b_info.type);
+ if (rc)
+ goto out;
dss->dsps.callback_device_model_done = soft_reset_dm_suspended;
libxl__domain_suspend_device_model(egc, &dss->dsps); /* must be last */
@@ -2204,6 +2207,8 @@ static void soft_reset_dm_suspended(libxl__egc *egc,
CONTAINER_OF(dsps, *srs, dss.dsps);
libxl__app_domain_create_state *cdcs = &srs->cdcs;
+ libxl__domain_suspend_dispose(gc, dsps);
+
/*
* Ask all backends to disconnect by removing the domain from
* xenstore. On the creation path the domain will be introduced to
diff --git a/tools/libxl/libxl_dom_suspend.c b/tools/libxl/libxl_dom_suspend.c
index 25d1571895..2a280f69a1 100644
--- a/tools/libxl/libxl_dom_suspend.c
+++ b/tools/libxl/libxl_dom_suspend.c
@@ -67,6 +67,16 @@ out:
return rc;
}
+void libxl__domain_suspend_dispose(libxl__gc *gc,
+ libxl__domain_suspend_state *dsps)
+{
+ libxl__xswait_stop(gc, &dsps->pvcontrol);
+ libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
+ libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
+ libxl__ev_time_deregister(gc, &dsps->guest_timeout);
+ libxl__ev_qmp_dispose(gc, &dsps->qmp);
+}
+
/*----- callbacks, called by xc_domain_save -----*/
void libxl__domain_suspend_device_model(libxl__egc *egc,
@@ -388,10 +398,7 @@ static void domain_suspend_common_done(libxl__egc *egc,
{
EGC_GC;
assert(!libxl__xswait_inuse(&dsps->pvcontrol));
- libxl__ev_evtchn_cancel(gc, &dsps->guest_evtchn);
- libxl__ev_xswatch_deregister(gc, &dsps->guest_watch);
- libxl__ev_time_deregister(gc, &dsps->guest_timeout);
- libxl__ev_qmp_dispose(gc, &dsps->qmp);
+ libxl__domain_suspend_dispose(gc, dsps);
dsps->callback_common_done(egc, dsps, rc);
}
diff --git a/tools/libxl/libxl_internal.h b/tools/libxl/libxl_internal.h
index 94a23179d3..3bc3bbcf84 100644
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3615,6 +3615,8 @@ struct libxl__domain_suspend_state {
int libxl__domain_suspend_init(libxl__egc *egc,
libxl__domain_suspend_state *dsps,
libxl_domain_type type);
+void libxl__domain_suspend_dispose(libxl__gc *gc,
+ libxl__domain_suspend_state *dsps);
/* calls dsps->callback_device_model_done when done
* may synchronously calls this callback */
--
2.30.1
^ permalink raw reply related [flat|nested] only message in thread
only message in thread, other threads:[~2021-03-18 13:57 UTC | newest]
Thread overview: (only message) (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-03-18 13:56 Xen Security Advisory 368 v3 (CVE-2021-28687) - HVM soft-reset crashes toolstack Xen.org security team
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.