Re: [PATCH 1/7] multiboot2: Add load type header and support for the PE binary type

["Roger Pau Monné" <roger.pau@citrix.com>]

On Tue, Mar 19, 2024 at 02:46:59PM +0000, Ross Lagerwall wrote:
> On Tue, Mar 19, 2024 at 1:18 PM Roger Pau Monné <roger.pau@citrix.com> wrote:
> >
> > On Wed, Mar 13, 2024 at 03:07:42PM +0000, Ross Lagerwall wrote:
> > > Currently, multiboot2-compatible bootloaders can load ELF binaries and
> > > a.out binaries. The presence of the address header tag determines
> > > how the bootloader tries to interpret the binary (a.out if the address
> > > tag is present else ELF).
> > >
> > > Add a new load type header tag that explicitly states the type of the
> > > binary. Bootloaders should use the binary type specified in the load
> > > type tag. If the load type tag is not present, the bootloader should
> > > fall back to the previous heuristics.
> > >
> > > In addition to the existing address and ELF load types, specify a new
> > > optional PE binary load type. This new type is a useful addition since
> > > PE binaries can be signed and verified (i.e. used with Secure Boot).
> > >
> > > Signed-off-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> > > ---
> > >  doc/multiboot.texi | 39 ++++++++++++++++++++++++++++++++++-----
> > >  doc/multiboot2.h   | 13 +++++++++++++
> > >  2 files changed, 47 insertions(+), 5 deletions(-)
> > >
> > > diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> > > index df8a0d056e76..d12719c744eb 100644
> > > --- a/doc/multiboot.texi
> > > +++ b/doc/multiboot.texi
> > > @@ -511,11 +511,12 @@ assumes that no bss segment is present.
> > >
> > >  Note: This information does not need to be provided if the kernel image
> > >  is in @sc{elf} format, but it must be provided if the image is in a.out
> > > -format or in some other format. When the address tag is present it must
> > > -be used in order to load the image, regardless of whether an @sc{elf}
> > > -header is also present. Compliant boot loaders must be able to load
> > > -images that are either in @sc{elf} format or contain the address tag
> > > -embedded in the Multiboot2 header.
> > > +format or in some other format. If the load type tag is not specified
> > > +and the address tag is present it must be used in order to load the
> > > +image, regardless of whether an @sc{elf} header is also present.
> > > +Compliant boot loaders must be able to load images that are either in
> > > +@sc{elf} format or contain the address tag embedded in the Multiboot2
> > > +header.
> > >
> > >  @subsection The entry address tag of Multiboot2 header
> > >
> > > @@ -732,6 +733,34 @@ and @samp{2} means load image at highest possible address but not
> > >  higher than max_addr.
> > >  @end table
> > >
> > > +@node Load type tag
> > > +@subsection Load type tag
> > > +
> > > +@example
> > > +@group
> > > +        +-------------------+
> > > +u16     | type = 11         |
> > > +u16     | flags             |
> > > +u32     | size = 12         |
> > > +u32     | load_type         |
> > > +        +-------------------+
> > > +@end group
> > > +@end example
> > > +
> > > +This tag indicates the type of the payload and how the boot loader
> > > +should load it.
> > > +
> > > +The meaning of each field is as follows:
> > > +
> > > +@table @code
> > > +@item load_type
> > > +Recognized load types are @samp{0} for address (i.e. load a.out using
> > > +the address tag), @samp{1} for ELF, and @samp{2} for PE. Compliant
> > > +bootloaders should implement support for a.out and ELF as a minimum.  If
> > > +this tag is not specified, the boot loader should attempt to load the
> > > +payload using the information specified in the address tag if present,
> > > +else it should load the payload as an ELF binary.  @end table
> >
> > I wonder if it would be simpler to use the following order instead:
> >
> > 1. Address tag
> > 2. Load type tag
> > 3. ELF header
> >
> > It's pointless to add a Loader type tag with load_type == 0, as that's
> > already mandated by the Address tag.  IOW: signaling the use of the
> > Address tag here is kind of pointless, if the fields in the Address
> > tag are set, that's the only signaling required for the data in the
> > Address tag to be used.
> >
> > Or are we attempting to support images that set Address tag and Load
> > type tag != 0 in order to use the Address tag when the loader doesn't
> > recognize the Load type tag, and otherwise use a different format?
> 
> No, that wasn't my intention. My intention for the load type tag was
> to have one tag that unambiguously describes the payload format and if
> that is missing, fall back to the previous logic for compatibility.
> It avoids more complicated heuristics if different payload types are
> added in the future.
> 
> >
> > Would it be sensible for multiboot2 to use PE in preference to ELF if
> > present on the image?  (without requiring any signaling).  I would
> > think this could be troublesome if kernels are so far expecting the
> > ELF header to be used with multiboot2, even if they also expose a PE
> > header.
> >
> 
> AFAIK an ELF image can't also be a valid PE/COFF image since they have
> different magic numbers at the start of the image. Perhaps it would
> be simpler to avoid introducing the load type tag and just load the
> payload based on the magic number?

I would likely be fine with just handling it like we handle ELF, if a
PE header is found use it.  As long as ELF and PE headers are mutually
exclusive.

Thanks, Roger.