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