diff mbox series

libxc: fix HVM core dump

Message ID 20190319160231.17120-1-wei.liu2@citrix.com (mailing list archive)
State Superseded
Headers show
Series libxc: fix HVM core dump | expand

Commit Message

Wei Liu March 19, 2019, 4:02 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.

Only issue the call when necessary in libxc.

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 | 18 +++++++++++-------
 1 file changed, 11 insertions(+), 7 deletions(-)

Comments

Ian Jackson March 20, 2019, 1:49 p.m. UTC | #1
Wei Liu writes ("[PATCH] 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.
...
> +    if ( !auto_translated_physmap &&
> +         xc_domain_get_guest_width(xch, domid, &dinfo->guest_width) != 0 )
> +    {
> +        PERROR("Could not get address size for domain");
> +        goto out;
> +    }

Did you consider and reject the option of writing:

    if ( auto_translated_physmap ) {
         ... xc_domain_get_guest_width(,,&dinfo->guest_width,,) ...
    } else {
         dinfo->guest_width = sizeof(unsigned long)
    }

This is perhaps less likely to leave dinfo->guest_width as an
uninitialised hazard.

Ian.
Wei Liu March 20, 2019, 3:40 p.m. UTC | #2
On Wed, Mar 20, 2019 at 01:49:30PM +0000, Ian Jackson wrote:
> Wei Liu writes ("[PATCH] 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.
> ...
> > +    if ( !auto_translated_physmap &&
> > +         xc_domain_get_guest_width(xch, domid, &dinfo->guest_width) != 0 )
> > +    {
> > +        PERROR("Could not get address size for domain");
> > +        goto out;
> > +    }
> 
> Did you consider and reject the option of writing:
> 
>     if ( auto_translated_physmap ) {
>          ... xc_domain_get_guest_width(,,&dinfo->guest_width,,) ...
>     } else {
>          dinfo->guest_width = sizeof(unsigned long)
>     }
> 
> This is perhaps less likely to leave dinfo->guest_width as an
> uninitialised hazard.

Works for me too.

Wei.

> 
> Ian.
diff mbox series

Patch

diff --git a/tools/libxc/xc_core.c b/tools/libxc/xc_core.c
index e581905ba9..c825b5b0a0 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,13 @@  xc_domain_dumpcore_via_callback(xc_interface *xch,
     }
     auto_translated_physmap = xc_core_arch_auto_translated_physmap(&info);
 
+    if ( !auto_translated_physmap &&
+         xc_domain_get_guest_width(xch, domid, &dinfo->guest_width) != 0 )
+    {
+        PERROR("Could not get address size for domain");
+        goto out;
+    }
+
     if ( domid != info.domid )
     {
         PERROR("Domain %d does not exist", domid);
@@ -742,7 +743,10 @@  xc_domain_dumpcore_via_callback(xc_interface *xch,
         goto out;
 
     /* elf note section: xen version */
-    sts = elfnote_dump_xen_version(xch, args, dump_rtn, dinfo->guest_width);
+    sts = elfnote_dump_xen_version(xch, args, dump_rtn,
+                                   auto_translated_physmap ?
+                                   sizeof(unsigned long):
+                                   dinfo->guest_width);
     if ( sts != 0 )
         goto out;