Re: [PATCH] libelf: Handle PVH kernels lacking ENTRY elfnote

[Jason Andryuk <jandryuk@xxxxxxxxx>]

On Thu, Oct 15, 2020 at 11:14 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
>
> On 15.10.2020 16:50, Jason Andryuk wrote:
> > On Thu, Oct 15, 2020 at 3:00 AM Jan Beulich <jbeulich@xxxxxxxx> wrote:
> >> And why is there no bounds check of ->phys_entry paralleling the
> >> ->virt_entry one?
> >
> > What is the purpose of this checking?  It's sanity checking which is
> > generally good, but what is the harm from failing the checks?  A
> > corrupt kernel can crash itself?  Maybe you could start executing
> > something (the initramfs?) instead of the actual kernel?
>
> This is at least getting close to a possible security issue.
> Booting a hacked up binary can be a problem afaik.

If you are already letting the user provide a kernel, they can give a
well formed kernel that does whatever they want.  Like Andrew wrote,
the concern would be if the binary can subvert the hypervisor/tools.

> >> On the whole, as long as we don't know what mode we're planning to
> >> boot in, we can't skip any checks, as the mere presence of
> >> XEN_ELFNOTE_PHYS32_ENTRY doesn't mean that's also what gets used.
> >> Therefore simply bypassing any of the checks is not an option.
> >
> > elf_xen_note_check() early exits when it finds phys_entry set, so
> > there is already some bypassing.
> >
> >> In
> >> particular what you suggest would lead to failure to check
> >> e_entry-derived ->virt_entry when the PVH-specific note is
> >> present but we're booting in PV mode. For now I don't see how to
> >> address this without making the function aware of the intended
> >> booting mode.
> >
> > Yes, the relevant checks depend on the desired booting mode.
> >
> > The e_entry use seems a little problematic.  You said the ELF
> > Specification states it should be a virtual address, but Linux seems
> > to fill it with a physical address.  You could use a heuristic e_entry
> > < 0 (0xffff...) to compare with the virtual addresses otherwise check
> > against physical?
>
> Don't we have a physical range as well? And don't we adjust the
> entry point already in certain cases anyway? Checking and adjustment
> can (and should) be brought in sync, and else checking the entry
> point fits at least one of the two ranges may be better than no
> checking at all, I think.

Looks like we can pass XC_DOM_PV_CONTAINER/XC_DOM_HVM_CONTAINER down
into elf_xen_parse().  Then we would just validate phys_entry for HVM
and virt_entry for PV.  Does that sound reasonable?

(The use in xc_dom_probe_hvm_kernel() is interesting to disallow
Xen-enabled kernel.)

Regards,
Jason