[Xen-devel] [PATCH] xl: Fix segfault on domain reboot

[Fatih Acar <fatih.acar@xxxxxxxxx>]

If we have no disk attached at startup, diskws is left unallocated
but `d_config.num_disks` may change if we attach a disk later.
When a domain is rebooted `evdisable_disk_ejects` is called
this will later result in a segfault if num_disks has changed.

Expand diskws when num_disks increases.

Signed-off-by: Fatih Acar <fatih.acar@xxxxxxxxx>
Signed-off-by: Nikita Kozlov <nikita.kozlov@xxxxxxxxx>
Signed-off-by: Vincent Legout <vincent.legout@xxxxxxxxx>
Signed-off-by: Baptiste Daroussin <baptiste.daroussin@xxxxxxxxx>
---
 tools/libxl/xl_cmdimpl.c | 34 ++++++++++++++++++++++++++++------
 1 file changed, 28 insertions(+), 6 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 7e8a8ae..f244e63 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -2517,13 +2517,25 @@ skip_usbdev:
     xlu_cfg_destroy(config);
 }
 
+static void realloc_diskws(libxl_evgen_disk_eject ***diskws, int old_count, 
int new_count)
+{
+    libxl_evgen_disk_eject **diskws_new;
+
+    diskws_new = xcalloc(new_count, sizeof(*diskws_new));
+    memcpy(diskws_new, *diskws, sizeof(**diskws) * old_count);
+    free(*diskws);
+    *diskws = diskws_new;
+}
+
 static void reload_domain_config(uint32_t domid,
-                                 libxl_domain_config *d_config)
+                                 libxl_domain_config *d_config,
+                                 libxl_evgen_disk_eject ***diskws)
 {
     int rc;
     uint8_t *t_data;
     int ret, t_len;
     libxl_domain_config d_config_new;
+    int disk_count;
 
     /* In case user has used "config-update" to store a new config
      * file.
@@ -2534,9 +2546,14 @@ static void reload_domain_config(uint32_t domid,
     }
     if (t_len > 0) {
         LOG("\"xl\" configuration found, using it\n");
+        disk_count = d_config->num_disks;
         libxl_domain_config_dispose(d_config);
         parse_config_data("<updated>", (const char *)t_data,
                           t_len, d_config);
+        if (d_config->num_disks > disk_count) {
+            /* reallocate bigger diskws */
+            realloc_diskws(diskws, disk_count, d_config->num_disks);
+        }
         free(t_data);
         libxl_userdata_unlink(ctx, domid, "xl");
         return;
@@ -2549,6 +2566,10 @@ static void reload_domain_config(uint32_t domid,
             "reusing old configuration", rc);
         libxl_domain_config_dispose(&d_config_new);
     } else {
+        if (d_config_new.num_disks > d_config->num_disks) {
+            /* reallocate bigger diskws */
+            realloc_diskws(diskws, d_config->num_disks, 
d_config_new.num_disks);
+        }
         libxl_domain_config_dispose(d_config);
         /* Steal allocations */
         memcpy(d_config, &d_config_new, sizeof(libxl_domain_config));
@@ -2558,7 +2579,8 @@ static void reload_domain_config(uint32_t domid,
 /* Can update r_domid if domain is destroyed */
 static domain_restart_type handle_domain_death(uint32_t *r_domid,
                                                libxl_event *event,
-                                               libxl_domain_config *d_config)
+                                               libxl_domain_config *d_config,
+                                               libxl_evgen_disk_eject 
***diskws)
 {
     domain_restart_type restart = DOMAIN_RESTART_NONE;
     libxl_action_on_shutdown action;
@@ -2614,12 +2636,12 @@ static domain_restart_type handle_domain_death(uint32_t 
*r_domid,
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART_RENAME:
-        reload_domain_config(*r_domid, d_config);
+        reload_domain_config(*r_domid, d_config, diskws);
         restart = DOMAIN_RESTART_RENAME;
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_RESTART:
-        reload_domain_config(*r_domid, d_config);
+        reload_domain_config(*r_domid, d_config, diskws);
         restart = DOMAIN_RESTART_NORMAL;
         /* fall-through */
     case LIBXL_ACTION_ON_SHUTDOWN_DESTROY:
@@ -2630,7 +2652,7 @@ static domain_restart_type handle_domain_death(uint32_t 
*r_domid,
         break;
 
     case LIBXL_ACTION_ON_SHUTDOWN_SOFT_RESET:
-        reload_domain_config(*r_domid, d_config);
+        reload_domain_config(*r_domid, d_config, diskws);
         restart = DOMAIN_RESTART_SOFT_RESET;
         break;
 
@@ -3137,7 +3159,7 @@ start:
             LOG("Domain %u has shut down, reason code %d 0x%x", domid,
                 event->u.domain_shutdown.shutdown_reason,
                 event->u.domain_shutdown.shutdown_reason);
-            switch (handle_domain_death(&domid, event, &d_config)) {
+            switch (handle_domain_death(&domid, event, &d_config, &diskws)) {
             case DOMAIN_RESTART_SOFT_RESET:
                 domid_soft_reset = domid;
                 domid = INVALID_DOMID;
-- 
bapt is my cto


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel