diff mbox series

[v2] libxc: fix HVM core dump

Message ID 20190320154338.24124-1-wei.liu2@citrix.com (mailing list archive)
State New, archived
Headers show
Series [v2] libxc: fix HVM core dump | expand

Commit Message

Wei Liu March 20, 2019, 3:43 p.m. UTC
f969bc9fc96 forbid get_address_size call on HVM guests, because that
didn't make sense. It broke core dump functionality on HVM because
libxc unconditionally asked for guest width.

Force guest_width to a sensible value.

Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
Signed-off-by: Wei Liu <wei.liu2@citrix.com>
---
Cc: Ian Jackson <ian.jackson@eu.citrix.com>
Cc: Jan Beulich <jbeulich@suse.com>
Cc: Andrew Cooper <andrew.cooper3@citrix.com>
Cc: Igor Druzhinin <igor.druzhinin@citrix.com>
Cc: Juergen Gross <jgross@suse.com>

Juergen, this is probably too late for 4.12, but you can at least add
a release note somewhere.

Ian, please backport this to 4.12.
---
 tools/libxc/xc_core.c | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

Comments

Ian Jackson March 20, 2019, 4:40 p.m. UTC | #1
Wei Liu writes ("[PATCH v2] libxc: fix HVM core dump"):
> f969bc9fc96 forbid get_address_size call on HVM guests, because that
> didn't make sense. It broke core dump functionality on HVM because
> libxc unconditionally asked for guest width.
> 
> Force guest_width to a sensible value.
> 
> Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>

LGTM.  Thanks for the copious comment.

Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>
Wei Liu March 20, 2019, 4:55 p.m. UTC | #2
On Wed, Mar 20, 2019 at 04:40:59PM +0000, Ian Jackson wrote:
> Wei Liu writes ("[PATCH v2] libxc: fix HVM core dump"):
> > f969bc9fc96 forbid get_address_size call on HVM guests, because that
> > didn't make sense. It broke core dump functionality on HVM because
> > libxc unconditionally asked for guest width.
> > 
> > Force guest_width to a sensible value.
> > 
> > Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> > Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> 
> LGTM.  Thanks for the copious comment.
> 
> Acked-by: Ian Jackson <ian.jackson@eu.citrix.com>

Thanks. Pushed.

Wei.
Jürgen Groß March 22, 2019, 12:06 p.m. UTC | #3
On 20/03/2019 16:43, Wei Liu wrote:
> f969bc9fc96 forbid get_address_size call on HVM guests, because that
> didn't make sense. It broke core dump functionality on HVM because
> libxc unconditionally asked for guest width.
> 
> Force guest_width to a sensible value.
> 
> Reported-by: Igor Druzhinin <igor.druzhinin@citrix.com>
> Signed-off-by: Wei Liu <wei.liu2@citrix.com>
> ---
> Cc: Ian Jackson <ian.jackson@eu.citrix.com>
> Cc: Jan Beulich <jbeulich@suse.com>
> Cc: Andrew Cooper <andrew.cooper3@citrix.com>
> Cc: Igor Druzhinin <igor.druzhinin@citrix.com>
> Cc: Juergen Gross <jgross@suse.com>
> 
> Juergen, this is probably too late for 4.12, but you can at least add
> a release note somewhere.
> 
> Ian, please backport this to 4.12.

As we have another blocker I'm fine with this patch for 4.12:

Release-acked-by: Juergen Gross <jgross@suse.com>


Juergen
diff mbox series

Patch

diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c
index e581905ba9..2ee1d205b4 100644
--- a/tools/libxc/xc_core.c
+++ b/tools/libxc/xc_core.c
@@ -459,12 +459,6 @@  xc_domain_dumpcore_via_callback(xc_interface *xch,
     struct xc_core_section_headers *sheaders = NULL;
     Elf64_Shdr *shdr;
  
-    if ( xc_domain_get_guest_width(xch, domid, &dinfo->guest_width) != 0 )
-    {
-        PERROR("Could not get address size for domain");
-        return sts;
-    }
-
     xc_core_arch_context_init(&arch_ctxt);
     if ( (dump_mem_start = malloc(DUMP_INCREMENT*PAGE_SIZE)) == NULL )
     {
@@ -487,6 +481,40 @@  xc_domain_dumpcore_via_callback(xc_interface *xch,
     }
     auto_translated_physmap = xc_core_arch_auto_translated_physmap(&info);
 
+    if ( !auto_translated_physmap )
+
+    {
+        if ( xc_domain_get_guest_width(xch, domid, &dinfo->guest_width) != 0 )
+        {
+            PERROR("Could not get address size for domain");
+            goto out;
+        }
+    }
+    else
+    {
+        /*
+         * Autotranslated guest never sets guest width in the first
+         * place. Force guest_width to be sizeof(unsigned long) so
+         * code below functions properly.
+         *
+         * Here is why this is correct.
+         *
+         * 1. Before f969bc9fc, xc_domain_get_guest_width for HVM (x86
+         * and ARM) always returned hypervisor's idea of
+         * sizeof(unsigned long).
+         *
+         * 2. There has never been a situation in which hypervisor's
+         * word width is smaller than toolstack domain's (i.e. no
+         * 32bit hypervisor + 64bit toolstack).
+         *
+         * Predicates in code test guest_width against toolstack
+         * domain's sizeof(unsigned long), so setting guest_width to
+         * toolstack domain's idea of sizeof(unsigned long) matches
+         * the original behaviour for HVM guests.
+         */
+        dinfo->guest_width = sizeof(unsigned long);
+    }
+
     if ( domid != info.domid )
     {
         PERROR("Domain %d does not exist", domid);