domain_destroy() is the wrong position to be freeing colouring information.
The comment in context identifies how domain_destroy() can be called multiple
times on the same domain, leading to a double free of d->llc_colors as it's
the wrong side of the atomic_cmpxchg() to be made safe.
Furthermore, by freeing d->llc_colors but leaving d->num_llc_colors nonzero,
alloc_color_heap_page() can in principle use after free.
Make domain_llc_coloring_free() idempotent, zeroing d->num_llc_colors and
NULL-ing d->llc_colors after freeing, and call it from domain_teardown().
Fixes: 6985aa5e0c3c ("xen: extend domctl interface for cache coloring")
Signed-off-by: Andrew Cooper <andrew.coop...@citrix.com>
---
CC: Anthony PERARD <anthony.per...@vates.tech>
CC: Michal Orzel <michal.or...@amd.com>
CC: Jan Beulich <jbeul...@suse.com>
CC: Julien Grall <jul...@xen.org>
CC: Roger Pau Monné <roger....@citrix.com>
CC: Stefano Stabellini <sstabell...@kernel.org>
CC: Carlo Nonato <carlo.non...@minervasys.tech>
CC: Marco Solieri <marco.soli...@minervasys.tech>
Cache colouring is experimental, which is why no XSA is being allocated for
these bugs.
---
xen/common/domain.c | 5 +++--
xen/common/llc-coloring.c | 4 +++-
2 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/xen/common/domain.c b/xen/common/domain.c
index 303c338ef293..4f79ba39878c 100644
--- a/xen/common/domain.c
+++ b/xen/common/domain.c
@@ -626,6 +626,9 @@ static int domain_teardown(struct domain *d)
case PROG_none:
BUILD_BUG_ON(PROG_none != 0);
+ /* Trivial teardown, not long-running enough to need a preemption
check. */
+ domain_llc_coloring_free(d);
+
PROGRESS(gnttab_mappings):
rc = gnttab_release_mappings(d);
if ( rc )
@@ -1447,8 +1450,6 @@ void domain_destroy(struct domain *d)
{
BUG_ON(!d->is_dying);
- domain_llc_coloring_free(d);
-
/* May be already destroyed, or get_domain() can race us. */
if ( atomic_cmpxchg(&d->refcnt, 0, DOMAIN_DESTROYED) != 0 )
return;
diff --git a/xen/common/llc-coloring.c b/xen/common/llc-coloring.c
index bd1f634f0bb8..ea3e0ca07017 100644
--- a/xen/common/llc-coloring.c
+++ b/xen/common/llc-coloring.c
@@ -309,8 +309,10 @@ int domain_set_llc_colors(struct domain *d,
void domain_llc_coloring_free(struct domain *d)
{
+ d->num_llc_colors = 0;
+
if ( d->llc_colors != default_colors )
- xfree(d->llc_colors);
+ XFREE(d->llc_colors);
}
int __init domain_set_llc_colors_from_str(struct domain *d, const char *str)
--
2.39.5
