* [PATCH v3 0/2] libelf: small fixes for PVH @ 2021-05-20 12:30 Roger Pau Monne 2021-05-20 12:30 ` [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest " Roger Pau Monne 2021-05-20 12:30 ` [PATCH v3 2/2] libelf: improve PVH elfnote parsing Roger Pau Monne 0 siblings, 2 replies; 7+ messages in thread From: Roger Pau Monne @ 2021-05-20 12:30 UTC (permalink / raw) To: xen-devel Cc: Roger Pau Monne, Andrew Cooper, George Dunlap, Ian Jackson, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu Hello, A couple of small fixes for PVH loading. The first one is likely not very relevant, since PVH couldn't be booted anyway with the data in the __xen_guest section, so it's mostly a cleanup. Second patch fixes the checks for PVH loading, as in that case physical addresses must always be used to perform the bound calculations. Thanks, Roger. Roger Pau Monne (2): libelf: don't attempt to parse __xen_guest for PVH libelf: improve PVH elfnote parsing tools/fuzz/libelf/libelf-fuzzer.c | 3 +- tools/libs/guest/xg_dom_elfloader.c | 6 ++-- tools/libs/guest/xg_dom_hvmloader.c | 2 +- xen/arch/x86/hvm/dom0_build.c | 2 +- xen/arch/x86/pv/dom0_build.c | 2 +- xen/common/libelf/libelf-dominfo.c | 49 +++++++++++++++++------------ xen/include/xen/libelf.h | 2 +- 7 files changed, 39 insertions(+), 27 deletions(-) -- 2.31.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest for PVH 2021-05-20 12:30 [PATCH v3 0/2] libelf: small fixes for PVH Roger Pau Monne @ 2021-05-20 12:30 ` Roger Pau Monne 2021-05-20 12:35 ` Jan Beulich 2021-05-20 12:30 ` [PATCH v3 2/2] libelf: improve PVH elfnote parsing Roger Pau Monne 1 sibling, 1 reply; 7+ messages in thread From: Roger Pau Monne @ 2021-05-20 12:30 UTC (permalink / raw) To: xen-devel Cc: Roger Pau Monne, Andrew Cooper, George Dunlap, Ian Jackson, Jan Beulich, Julien Grall, Stefano Stabellini, Wei Liu The legacy __xen_guest section doesn't support the PHYS32_ENTRY elfnote, so it's pointless to attempt to parse the elfnotes from that section when called from an hvm container. Suggested-by: Jan Beulich <jbeulich@suse.com> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v2: - New in this version. --- xen/common/libelf/libelf-dominfo.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c index 69c94b6f3bb..abea1011c18 100644 --- a/xen/common/libelf/libelf-dominfo.c +++ b/xen/common/libelf/libelf-dominfo.c @@ -577,10 +577,8 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, } - /* - * Finally fall back to the __xen_guest section. - */ - if ( xen_elfnotes == 0 ) + /* Finally fall back to the __xen_guest section for PV guests only. */ + if ( xen_elfnotes == 0 && !hvm ) { shdr = elf_shdr_by_name(elf, "__xen_guest"); if ( ELF_HANDLE_VALID(shdr) ) -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest for PVH 2021-05-20 12:30 ` [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest " Roger Pau Monne @ 2021-05-20 12:35 ` Jan Beulich 2021-05-21 13:31 ` Jan Beulich 0 siblings, 1 reply; 7+ messages in thread From: Jan Beulich @ 2021-05-20 12:35 UTC (permalink / raw) To: Roger Pau Monne Cc: Andrew Cooper, George Dunlap, Ian Jackson, Julien Grall, Stefano Stabellini, Wei Liu, xen-devel On 20.05.2021 14:30, Roger Pau Monne wrote: > The legacy __xen_guest section doesn't support the PHYS32_ENTRY > elfnote, so it's pointless to attempt to parse the elfnotes from that > section when called from an hvm container. > > Suggested-by: Jan Beulich <jbeulich@suse.com> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> > --- > Changes since v2: > - New in this version. > --- > xen/common/libelf/libelf-dominfo.c | 6 ++---- > 1 file changed, 2 insertions(+), 4 deletions(-) > > diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c > index 69c94b6f3bb..abea1011c18 100644 > --- a/xen/common/libelf/libelf-dominfo.c > +++ b/xen/common/libelf/libelf-dominfo.c > @@ -577,10 +577,8 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, > > } > > - /* > - * Finally fall back to the __xen_guest section. > - */ > - if ( xen_elfnotes == 0 ) > + /* Finally fall back to the __xen_guest section for PV guests only. */ > + if ( xen_elfnotes == 0 && !hvm ) Isn't this depending on the 2nd patch adding the function parameter? IOW doesn't this break the build, even if just transiently? With the respective hunk from patch 2 moved here Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest for PVH 2021-05-20 12:35 ` Jan Beulich @ 2021-05-21 13:31 ` Jan Beulich 2021-05-28 7:21 ` Jan Beulich 0 siblings, 1 reply; 7+ messages in thread From: Jan Beulich @ 2021-05-21 13:31 UTC (permalink / raw) To: Roger Pau Monne Cc: Andrew Cooper, George Dunlap, Ian Jackson, Julien Grall, Stefano Stabellini, Wei Liu, xen-devel On 20.05.2021 14:35, Jan Beulich wrote: > On 20.05.2021 14:30, Roger Pau Monne wrote: >> The legacy __xen_guest section doesn't support the PHYS32_ENTRY >> elfnote, so it's pointless to attempt to parse the elfnotes from that >> section when called from an hvm container. >> >> Suggested-by: Jan Beulich <jbeulich@suse.com> >> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >> --- >> Changes since v2: >> - New in this version. >> --- >> xen/common/libelf/libelf-dominfo.c | 6 ++---- >> 1 file changed, 2 insertions(+), 4 deletions(-) >> >> diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c >> index 69c94b6f3bb..abea1011c18 100644 >> --- a/xen/common/libelf/libelf-dominfo.c >> +++ b/xen/common/libelf/libelf-dominfo.c >> @@ -577,10 +577,8 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, >> >> } >> >> - /* >> - * Finally fall back to the __xen_guest section. >> - */ >> - if ( xen_elfnotes == 0 ) >> + /* Finally fall back to the __xen_guest section for PV guests only. */ >> + if ( xen_elfnotes == 0 && !hvm ) > > Isn't this depending on the 2nd patch adding the function parameter? > IOW doesn't this break the build, even if just transiently? With the > respective hunk from patch 2 moved here > Reviewed-by: Jan Beulich <jbeulich@suse.com> With the intention of committing I did this hunk movement, noticing that - it's more than just one hunk, - a tool stack maintainer ack is actually going to be needed (all respective hunks are now in the first patch). I'll keep the massaged patches locally, until the missing ack arrives. Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest for PVH 2021-05-21 13:31 ` Jan Beulich @ 2021-05-28 7:21 ` Jan Beulich 0 siblings, 0 replies; 7+ messages in thread From: Jan Beulich @ 2021-05-28 7:21 UTC (permalink / raw) To: Ian Jackson, Wei Liu Cc: Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, xen-devel, Roger Pau Monne On 21.05.2021 15:31, Jan Beulich wrote: > On 20.05.2021 14:35, Jan Beulich wrote: >> On 20.05.2021 14:30, Roger Pau Monne wrote: >>> The legacy __xen_guest section doesn't support the PHYS32_ENTRY >>> elfnote, so it's pointless to attempt to parse the elfnotes from that >>> section when called from an hvm container. >>> >>> Suggested-by: Jan Beulich <jbeulich@suse.com> >>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> >>> --- >>> Changes since v2: >>> - New in this version. >>> --- >>> xen/common/libelf/libelf-dominfo.c | 6 ++---- >>> 1 file changed, 2 insertions(+), 4 deletions(-) >>> >>> diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c >>> index 69c94b6f3bb..abea1011c18 100644 >>> --- a/xen/common/libelf/libelf-dominfo.c >>> +++ b/xen/common/libelf/libelf-dominfo.c >>> @@ -577,10 +577,8 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, >>> >>> } >>> >>> - /* >>> - * Finally fall back to the __xen_guest section. >>> - */ >>> - if ( xen_elfnotes == 0 ) >>> + /* Finally fall back to the __xen_guest section for PV guests only. */ >>> + if ( xen_elfnotes == 0 && !hvm ) >> >> Isn't this depending on the 2nd patch adding the function parameter? >> IOW doesn't this break the build, even if just transiently? With the >> respective hunk from patch 2 moved here >> Reviewed-by: Jan Beulich <jbeulich@suse.com> > > With the intention of committing I did this hunk movement, noticing > that > - it's more than just one hunk, > - a tool stack maintainer ack is actually going to be needed (all > respective hunks are now in the first patch). > I'll keep the massaged patches locally, until the missing ack arrives. I've timed out waiting for an ack and committed the patches, considering the tool stack parts of them are mechanical enough. Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v3 2/2] libelf: improve PVH elfnote parsing 2021-05-20 12:30 [PATCH v3 0/2] libelf: small fixes for PVH Roger Pau Monne 2021-05-20 12:30 ` [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest " Roger Pau Monne @ 2021-05-20 12:30 ` Roger Pau Monne 2021-05-20 12:37 ` Jan Beulich 1 sibling, 1 reply; 7+ messages in thread From: Roger Pau Monne @ 2021-05-20 12:30 UTC (permalink / raw) To: xen-devel Cc: Roger Pau Monne, Ian Jackson, Wei Liu, Andrew Cooper, George Dunlap, Jan Beulich, Julien Grall, Stefano Stabellini Pass an hvm boolean parameter to the elf note parsing and checking routines, so that better checking can be done in case libelf is dealing with an hvm container. elf_xen_note_check shouldn't return early unless PHYS32_ENTRY is set and the container is of type HVM, or else the loader and version checks would be avoided for kernels intended to be booted as PV but that also have PHYS32_ENTRY set. Adjust elf_xen_addr_calc_check so that the virtual addresses are actually physical ones (by setting virt_base and elf_paddr_offset to zero) when the container is of type HVM, as that container is always started with paging disabled. Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> --- Changes since v2: - Do not print messages about VIRT_BASE/ELF_PADDR_OFFSET unset on HVM. - Make it explicit that elf_paddr_offset will be set to 0 regardless of elf_note_start value for HVM. Changes since v1: - Expand comments. - Do not set virt_entry to phys_entry unless it's an HVM container. --- tools/fuzz/libelf/libelf-fuzzer.c | 3 +- tools/libs/guest/xg_dom_elfloader.c | 6 ++-- tools/libs/guest/xg_dom_hvmloader.c | 2 +- xen/arch/x86/hvm/dom0_build.c | 2 +- xen/arch/x86/pv/dom0_build.c | 2 +- xen/common/libelf/libelf-dominfo.c | 43 ++++++++++++++++++----------- xen/include/xen/libelf.h | 2 +- 7 files changed, 37 insertions(+), 23 deletions(-) diff --git a/tools/fuzz/libelf/libelf-fuzzer.c b/tools/fuzz/libelf/libelf-fuzzer.c index 1ba85717114..84fb84720fa 100644 --- a/tools/fuzz/libelf/libelf-fuzzer.c +++ b/tools/fuzz/libelf/libelf-fuzzer.c @@ -17,7 +17,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) return -1; elf_parse_binary(elf); - elf_xen_parse(elf, &parms); + elf_xen_parse(elf, &parms, false); + elf_xen_parse(elf, &parms, true); return 0; } diff --git a/tools/libs/guest/xg_dom_elfloader.c b/tools/libs/guest/xg_dom_elfloader.c index 06e713fe111..ad71163dd92 100644 --- a/tools/libs/guest/xg_dom_elfloader.c +++ b/tools/libs/guest/xg_dom_elfloader.c @@ -135,7 +135,8 @@ static elf_negerrnoval xc_dom_probe_elf_kernel(struct xc_dom_image *dom) * or else we might be trying to load a plain ELF. */ elf_parse_binary(&elf); - rc = elf_xen_parse(&elf, dom->parms); + rc = elf_xen_parse(&elf, dom->parms, + dom->container_type == XC_DOM_HVM_CONTAINER); if ( rc != 0 ) return rc; @@ -166,7 +167,8 @@ static elf_negerrnoval xc_dom_parse_elf_kernel(struct xc_dom_image *dom) /* parse binary and get xen meta info */ elf_parse_binary(elf); - if ( elf_xen_parse(elf, dom->parms) != 0 ) + if ( elf_xen_parse(elf, dom->parms, + dom->container_type == XC_DOM_HVM_CONTAINER) != 0 ) { rc = -EINVAL; goto out; diff --git a/tools/libs/guest/xg_dom_hvmloader.c b/tools/libs/guest/xg_dom_hvmloader.c index ec6ebad7fd5..3a63b23ba39 100644 --- a/tools/libs/guest/xg_dom_hvmloader.c +++ b/tools/libs/guest/xg_dom_hvmloader.c @@ -73,7 +73,7 @@ static elf_negerrnoval xc_dom_probe_hvm_kernel(struct xc_dom_image *dom) * else we might be trying to load a PV kernel. */ elf_parse_binary(&elf); - rc = elf_xen_parse(&elf, dom->parms); + rc = elf_xen_parse(&elf, dom->parms, true); if ( rc == 0 ) return -EINVAL; diff --git a/xen/arch/x86/hvm/dom0_build.c b/xen/arch/x86/hvm/dom0_build.c index 878dc1d808e..c24b9efdb0a 100644 --- a/xen/arch/x86/hvm/dom0_build.c +++ b/xen/arch/x86/hvm/dom0_build.c @@ -561,7 +561,7 @@ static int __init pvh_load_kernel(struct domain *d, const module_t *image, elf_set_verbose(&elf); #endif elf_parse_binary(&elf); - if ( (rc = elf_xen_parse(&elf, &parms)) != 0 ) + if ( (rc = elf_xen_parse(&elf, &parms, true)) != 0 ) { printk("Unable to parse kernel for ELFNOTES\n"); return rc; diff --git a/xen/arch/x86/pv/dom0_build.c b/xen/arch/x86/pv/dom0_build.c index e0801a9e6d1..af47615b226 100644 --- a/xen/arch/x86/pv/dom0_build.c +++ b/xen/arch/x86/pv/dom0_build.c @@ -353,7 +353,7 @@ int __init dom0_construct_pv(struct domain *d, elf_set_verbose(&elf); elf_parse_binary(&elf); - if ( (rc = elf_xen_parse(&elf, &parms)) != 0 ) + if ( (rc = elf_xen_parse(&elf, &parms, false)) != 0 ) goto out; /* compatibility check */ diff --git a/xen/common/libelf/libelf-dominfo.c b/xen/common/libelf/libelf-dominfo.c index abea1011c18..24d1371dd7a 100644 --- a/xen/common/libelf/libelf-dominfo.c +++ b/xen/common/libelf/libelf-dominfo.c @@ -360,7 +360,7 @@ elf_errorstatus elf_xen_parse_guest_info(struct elf_binary *elf, /* sanity checks */ static elf_errorstatus elf_xen_note_check(struct elf_binary *elf, - struct elf_dom_parms *parms) + struct elf_dom_parms *parms, bool hvm) { if ( (ELF_PTRVAL_INVALID(parms->elf_note_start)) && (ELF_PTRVAL_INVALID(parms->guest_info)) ) @@ -382,7 +382,7 @@ static elf_errorstatus elf_xen_note_check(struct elf_binary *elf, } /* PVH only requires one ELF note to be set */ - if ( parms->phys_entry != UNSET_ADDR32 ) + if ( parms->phys_entry != UNSET_ADDR32 && hvm ) { elf_msg(elf, "ELF: Found PVH image\n"); return 0; @@ -414,7 +414,7 @@ static elf_errorstatus elf_xen_note_check(struct elf_binary *elf, } static elf_errorstatus elf_xen_addr_calc_check(struct elf_binary *elf, - struct elf_dom_parms *parms) + struct elf_dom_parms *parms, bool hvm) { uint64_t virt_offset; @@ -425,12 +425,15 @@ static elf_errorstatus elf_xen_addr_calc_check(struct elf_binary *elf, return -1; } - /* Initial guess for virt_base is 0 if it is not explicitly defined. */ - if ( parms->virt_base == UNSET_ADDR ) + /* + * Initial guess for virt_base is 0 if it is not explicitly defined in the + * PV case. For PVH virt_base is forced to 0 because paging is disabled. + */ + if ( parms->virt_base == UNSET_ADDR || hvm ) { parms->virt_base = 0; - elf_msg(elf, "ELF: VIRT_BASE unset, using %#" PRIx64 "\n", - parms->virt_base); + if ( !hvm ) + elf_msg(elf, "ELF: VIRT_BASE unset, using 0\n"); } /* @@ -441,23 +444,31 @@ static elf_errorstatus elf_xen_addr_calc_check(struct elf_binary *elf, * * If we are using the modern ELF notes interface then the default * is 0. + * + * For PVH this is forced to 0, as it's already a legacy option for PV. */ - if ( parms->elf_paddr_offset == UNSET_ADDR ) + if ( parms->elf_paddr_offset == UNSET_ADDR || hvm ) { - if ( parms->elf_note_start ) + if ( parms->elf_note_start || hvm ) parms->elf_paddr_offset = 0; else parms->elf_paddr_offset = parms->virt_base; - elf_msg(elf, "ELF_PADDR_OFFSET unset, using %#" PRIx64 "\n", - parms->elf_paddr_offset); + if ( !hvm ) + elf_msg(elf, "ELF_PADDR_OFFSET unset, using %#" PRIx64 "\n", + parms->elf_paddr_offset); } virt_offset = parms->virt_base - parms->elf_paddr_offset; parms->virt_kstart = elf->pstart + virt_offset; parms->virt_kend = elf->pend + virt_offset; - if ( parms->virt_entry == UNSET_ADDR ) - parms->virt_entry = elf_uval(elf, elf->ehdr, e_entry); + if ( parms->virt_entry == UNSET_ADDR || hvm ) + { + if ( parms->phys_entry != UNSET_ADDR32 && hvm ) + parms->virt_entry = parms->phys_entry; + else + parms->virt_entry = elf_uval(elf, elf->ehdr, e_entry); + } if ( parms->bsd_symtab ) { @@ -499,7 +510,7 @@ static elf_errorstatus elf_xen_addr_calc_check(struct elf_binary *elf, /* glue it all together ... */ elf_errorstatus elf_xen_parse(struct elf_binary *elf, - struct elf_dom_parms *parms) + struct elf_dom_parms *parms, bool hvm) { ELF_HANDLE_DECL(elf_shdr) shdr; ELF_HANDLE_DECL(elf_phdr) phdr; @@ -592,9 +603,9 @@ elf_errorstatus elf_xen_parse(struct elf_binary *elf, } } - if ( elf_xen_note_check(elf, parms) != 0 ) + if ( elf_xen_note_check(elf, parms, hvm) != 0 ) return -1; - if ( elf_xen_addr_calc_check(elf, parms) != 0 ) + if ( elf_xen_addr_calc_check(elf, parms, hvm) != 0 ) return -1; return 0; } diff --git a/xen/include/xen/libelf.h b/xen/include/xen/libelf.h index b73998150fc..be47b0cc366 100644 --- a/xen/include/xen/libelf.h +++ b/xen/include/xen/libelf.h @@ -454,7 +454,7 @@ int elf_xen_parse_note(struct elf_binary *elf, int elf_xen_parse_guest_info(struct elf_binary *elf, struct elf_dom_parms *parms); int elf_xen_parse(struct elf_binary *elf, - struct elf_dom_parms *parms); + struct elf_dom_parms *parms, bool hvm); static inline void *elf_memcpy_unchecked(void *dest, const void *src, size_t n) { return memcpy(dest, src, n); } -- 2.31.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v3 2/2] libelf: improve PVH elfnote parsing 2021-05-20 12:30 ` [PATCH v3 2/2] libelf: improve PVH elfnote parsing Roger Pau Monne @ 2021-05-20 12:37 ` Jan Beulich 0 siblings, 0 replies; 7+ messages in thread From: Jan Beulich @ 2021-05-20 12:37 UTC (permalink / raw) To: Roger Pau Monne Cc: Ian Jackson, Wei Liu, Andrew Cooper, George Dunlap, Julien Grall, Stefano Stabellini, xen-devel On 20.05.2021 14:30, Roger Pau Monne wrote: > Pass an hvm boolean parameter to the elf note parsing and checking > routines, so that better checking can be done in case libelf is > dealing with an hvm container. > > elf_xen_note_check shouldn't return early unless PHYS32_ENTRY is set > and the container is of type HVM, or else the loader and version > checks would be avoided for kernels intended to be booted as PV but > that also have PHYS32_ENTRY set. > > Adjust elf_xen_addr_calc_check so that the virtual addresses are > actually physical ones (by setting virt_base and elf_paddr_offset to > zero) when the container is of type HVM, as that container is always > started with paging disabled. > > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com> With the one hunk moved to patch 1 Reviewed-by: Jan Beulich <jbeulich@suse.com> Jan ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-05-28 7:22 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-05-20 12:30 [PATCH v3 0/2] libelf: small fixes for PVH Roger Pau Monne 2021-05-20 12:30 ` [PATCH v3 1/2] libelf: don't attempt to parse __xen_guest " Roger Pau Monne 2021-05-20 12:35 ` Jan Beulich 2021-05-21 13:31 ` Jan Beulich 2021-05-28 7:21 ` Jan Beulich 2021-05-20 12:30 ` [PATCH v3 2/2] libelf: improve PVH elfnote parsing Roger Pau Monne 2021-05-20 12:37 ` Jan Beulich
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.