Re: [Xen-devel] [PATCH] libelf: allow having HYPERCALL_PAGE entry before VIRT_BASE in __xen_guest section.

[Gregory Herrero <gregory.herrero@xxxxxxxxxx>]

On Wed, Oct 11, 2017 at 06:40:14AM -0600, Jan Beulich wrote:
> >>> On 11.10.17 at 14:03, <gregory.herrero@xxxxxxxxxx> wrote:
> > When filling __xen_guest section of a guest, user may define
> > HYPERCALL_PAGE earlier than VIRT_BASE in the section leading to an
> > incorrect hypercall page address since an undefined virt_base could be
> > used to compute hypercall page address.
> > If there is no VIRT_BASE entry in __xen_guest section, default value of
> > 0 is used for virt_base. Thus, setting hypercall page address to
> > HYPERCALL_PAGE value is correct in this case too.
> 
> Do you have an example of a guest kernel that is doing this? I
> ask because the __xen_guest section has been deprecated for
> many years - everyone should be using the notes section
> instead nowadays.
> 
There is no public example AFAIK, I faced this issue while creating a
guest locally for testing purpose.

> > --- a/xen/common/libelf/libelf-dominfo.c
> > +++ b/xen/common/libelf/libelf-dominfo.c
> > @@ -330,14 +330,21 @@ elf_errorstatus elf_xen_parse_guest_info(struct 
> > elf_binary *elf,
> >  
> >          /* longs */
> >          if ( !strcmp(name, "VIRT_BASE") )
> > +        {
> >              parms->virt_base = strtoull(value, NULL, 0);
> > +            if ( parms->virt_hypercall != UNSET_ADDR )
> > +                params->virt_hypercall += params->virt_base;
> > +        }
> >          if ( !strcmp(name, "VIRT_ENTRY") )
> >              parms->virt_entry = strtoull(value, NULL, 0);
> >          if ( !strcmp(name, "ELF_PADDR_OFFSET") )
> >              parms->elf_paddr_offset = strtoull(value, NULL, 0);
> >          if ( !strcmp(name, "HYPERCALL_PAGE") )
> > -            parms->virt_hypercall = (strtoull(value, NULL, 0) << 12) +
> > -                parms->virt_base;
> > +        {
> > +            parms->virt_hypercall = (strtoull(value, NULL, 0) << 12);
> > +            if ( parms->virt_base != UNSET_ADDR )
> > +                params->virt_hypercall += params->virt_base;
> > +        }
> 
> I think you rather want to do this once after the loop. That'll
> then also take care of there being multiple VIRT_BASE entries.
> 
I will send an updated patch with your suggestion. Feel free to ignore
it if usage of __xen_guest section is deprecated.

Thanks,
Gregory

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel