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'.

This is XSA-368.

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>
master commit: dae3c3e8b257cd27d6b35a467a34bf79a6650340
master date: 2021-03-18 14:56:33 +0100

diff --git a/tools/libxl/libxl_create.c b/tools/libxl/libxl_create.c
index a4e74a5cd2bbdc770a46f6675b6d03f662c46d9e..f436a967aa9827581d840980b3d365c98a23f4ff 100644 (file)
--- a/tools/libxl/libxl_create.c
+++ b/tools/libxl/libxl_create.c
@@ -1848,9 +1848,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);
@@ -1860,6 +1858,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 */
 
@@ -1878,6 +1881,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 d1af3a65730387ee32be4dcaa70fb096cae9bacb..8e7c3464ae6154a24bafa14f77f559f675d49cb2 100644 (file)
--- 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,
@@ -393,10 +403,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 a119de713671965737dacf8fc4cc5bc2d13bc75b..6e1caff2c360df82398425d81fffbcb11e9c6a75 100644 (file)
--- a/tools/libxl/libxl_internal.h
+++ b/tools/libxl/libxl_internal.h
@@ -3443,6 +3443,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 */