[PATCH] libxc: fix HVM core dump

[PATCH] libxc: fix HVM core dump

[Wei Liu]

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(-)

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;
 
-- 
2.20.1


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc: fix HVM core dump

[Ian Jackson]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

Re: [PATCH] libxc: fix HVM core dump

[Wei Liu]

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.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel