[PATCH 2/4] xen/cache-col: Fix freeing of colouring information

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

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.cooper3@xxxxxxxxxx>
---
CC: Anthony PERARD <anthony.perard@xxxxxxxxxx>
CC: Michal Orzel <michal.orzel@xxxxxxx>
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Julien Grall <julien@xxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
CC: Carlo Nonato <carlo.nonato@xxxxxxxxxxxxxxx>
CC: Marco Solieri <marco.solieri@xxxxxxxxxxxxxxx>

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