Re: [PATCH 2/7] multiboot2: Allow 64-bit entry tags

2024-03-28 Thread Roger Pau Monné via Grub-devel
On Thu, Mar 28, 2024 at 03:05:47PM +, Ross Lagerwall wrote:
> On Tue, Mar 19, 2024 at 10:07 AM Roger Pau Monné  wrote:
> >
> > On Wed, Mar 13, 2024 at 03:07:43PM +, Ross Lagerwall wrote:
> > > Binaries may be built with entry points above 4G. While bootloaders may
> > > relocate them below 4G, it should be possible for the binary to specify
> > > those entry points. Therefore, extend the multiboot2 protocol such that
> > > 64 bit addresses are allowed for entry points. The extension is done in
> > > a backwards-compatible way.
> > >
> > > Signed-off-by: Ross Lagerwall 
> > > ---
> > >  doc/multiboot.texi | 32 +++-
> > >  doc/multiboot2.h   |  6 +-
> > >  2 files changed, 24 insertions(+), 14 deletions(-)
> > >
> > > diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> > > index d12719c744eb..049afab53c1f 100644
> > > --- a/doc/multiboot.texi
> > > +++ b/doc/multiboot.texi
> > > @@ -522,12 +522,12 @@ header.
> > >
> > >  @example
> > >  @group
> > > -+---+
> > > -u16 | type = 3  |
> > > -u16 | flags |
> > > -u32 | size  |
> > > -u32 | entry_addr|
> > > -+---+
> > > +  +---+
> > > +u16   | type = 3  |
> > > +u16   | flags |
> > > +u32   | size  |
> > > +u32 / u64 | entry_addr|
> > > +  +---+
> >
> > I might be confused, but this entry point is used in 32bit protected
> > mode, and hence a 64bit value is simply impossible to use according to
> > the protocol in "3.3 I386 machine state".
> >
> > Unless that section is expanded to describe other protocols that use
> > the entry address in a way where 64bits could be meaningful it seems
> > pointless to expand the field.
> 
> I changed this because the same binary is being used for both BIOS boot
> and UEFI boot, therefore it may have a base address above 4 GiB.
> Despite that, it is expected that GRUB would relocate the binary below
> 4 GiB so BIOS boot would still work.

Right, for UEFI boot it's possible to have entry addresses above 4GB,
because the entry point is called in long mode with identity page
tables (and hence you can put addresses in %rip past the 4GB
boundary).

However the multiboot entry point puts the CPU in 32bit protected
mode, and hence %eip can only hold a value below the 4GB boundary.

It's technically impossible to use an entry point above 4GB, unless
there's something that I'm missing that changes the initial CPU state
for the multiboot2 entry point.

Thanks, Roger.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2024-03-20 Thread Roger Pau Monné via Grub-devel
On Tue, Mar 19, 2024 at 02:46:59PM +, Ross Lagerwall wrote:
> On Tue, Mar 19, 2024 at 1:18 PM Roger Pau Monné  wrote:
> >
> > On Wed, Mar 13, 2024 at 03:07:42PM +, 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 
> > > ---
> > >  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 

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

2024-03-19 Thread Roger Pau Monné via Grub-devel
On Wed, Mar 13, 2024 at 03:07:42PM +, 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 
> ---
>  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?

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.

Thanks, Roger.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


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

2024-03-19 Thread Roger Pau Monné via Grub-devel
On Thu, Mar 14, 2024 at 02:24:31PM +, Ross Lagerwall wrote:
> On Thu, Mar 14, 2024 at 1:37 PM Jan Beulich  wrote:
> >
> > On 14.03.2024 10:30, Ross Lagerwall wrote:
> > > On Thu, Mar 14, 2024 at 7:24 AM Jan Beulich  wrote:
> > >>
> > >> On 13.03.2024 16:07, Ross Lagerwall wrote:
> > >>> 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).
> > >>
> > >> And the consideration to have ELF signable (by whatever extension to
> > >> the ELF spec) went nowhere?
> > >>
> > >
> > > I'm not sure if you're referring to some ongoing work to create signable
> > > ELFs that I'm not aware of.
> >
> > Something must have been invented already to make Linux modules signable.
> 
> Linux module signatures operate outside of the ELF container. In fact,
> AFAIK the actual signed content could be anything. The file format is:
> 
> * Content (i.e. ELF binary)
> * Signature of content in PKCS7 format
> * Signature info, including signature length
> * Magic marker: "~Module signature appended~\n"
> 
> This kind of arrangement does indeed work but it is fragile. Since the
> signature is on the entire contents and tools that understand ELF don't
> parse the signature, any transformation of the binary (e.g. to
> strip out debuginfo) will cause the signature to be lost / invalidated.
> 
> Nevertheless, this could still be an option for Xen if this is
> deemed to be a preferred solution by others. It would be good to hear
> some opinions on this.

No, IMO the PE route is likely the best one, as there's already all
the tooling around it, and it's what other OSes use to perform secure
boot.

It would have been nice for ELF to grow an extension to the spec for
image integrity data, but I don't see myself doing the work TBH.

Thanks, Roger.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH 2/7] multiboot2: Allow 64-bit entry tags

2024-03-19 Thread Roger Pau Monné via Grub-devel
On Wed, Mar 13, 2024 at 03:07:43PM +, Ross Lagerwall wrote:
> Binaries may be built with entry points above 4G. While bootloaders may
> relocate them below 4G, it should be possible for the binary to specify
> those entry points. Therefore, extend the multiboot2 protocol such that
> 64 bit addresses are allowed for entry points. The extension is done in
> a backwards-compatible way.
> 
> Signed-off-by: Ross Lagerwall 
> ---
>  doc/multiboot.texi | 32 +++-
>  doc/multiboot2.h   |  6 +-
>  2 files changed, 24 insertions(+), 14 deletions(-)
> 
> diff --git a/doc/multiboot.texi b/doc/multiboot.texi
> index d12719c744eb..049afab53c1f 100644
> --- a/doc/multiboot.texi
> +++ b/doc/multiboot.texi
> @@ -522,12 +522,12 @@ header.
>  
>  @example
>  @group
> -+---+
> -u16 | type = 3  |
> -u16 | flags |
> -u32 | size  |
> -u32 | entry_addr|
> -+---+
> +  +---+
> +u16   | type = 3  |
> +u16   | flags |
> +u32   | size  |
> +u32 / u64 | entry_addr|
> +  +---+

I might be confused, but this entry point is used in 32bit protected
mode, and hence a 64bit value is simply impossible to use according to
the protocol in "3.3 I386 machine state".

Unless that section is expanded to describe other protocols that use
the entry address in a way where 64bits could be meaningful it seems
pointless to expand the field.

>  @end group
>  @end example
>  
> @@ -538,7 +538,10 @@ The meaning of each is as follows:
>  
>  @item entry_addr
>  The physical address to which the boot loader should jump in order to
> -start running the operating system.
> +start running the operating system. @samp{entry_addr} may be specified
> +either as a @samp{u32} or @samp{u64}. The bootloader should use the
> +header size to determine the size of @samp{entry_addr}.
> +
>  @end table
>  
>  @subsection EFI i386 entry address tag of Multiboot2 header
> @@ -573,12 +576,12 @@ tag of Multiboot2 header are ignored.
>  
>  @example
>  @group
> -+---+
> -u16 | type = 9  |
> -u16 | flags |
> -u32 | size  |
> -u32 | entry_addr|
> -+---+
> +  +---+
> +u16   | type = 9  |
> +u16   | flags |
> +u32   | size  |
> +u32 / u64 | entry_addr|
> +  +---+

This does seem sensible.

Thanks, Roger.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: [PATCH] lib/relocator: always enforce the requested alignment in malloc_in_range()

2023-05-08 Thread Roger Pau Monné via Grub-devel
On Fri, Apr 28, 2023 at 04:15:24PM +0200, Daniel Kiper wrote:
> On Thu, Apr 27, 2023 at 05:06:54PM +0200, Roger Pau Monne via Grub-devel 
> wrote:
> > On failure to allocate from grub_relocator_firmware_alloc_region() in
> > malloc_in_range() the function would stop enforcing the alignment, and
> > the following was returned:
> >
> > lib/relocator.c:431: trying to allocate in 0x20-0xffbf9fff aligned 
> > 0x20 size 0x406000
> > lib/relocator.c:1197: allocated: 0x74de2000+0x406000
> > lib/relocator.c:1407: allocated 0x74de2000/0x74de2000
> >
> > Fix this by making sure that target always contains a suitably aligned
> > address.  After the change the return from the function is:
> >
> > lib/relocator.c:431: trying to allocate in 0x20-0xffb87fff aligned 
> > 0x20 size 0x478000
> > lib/relocator.c:1204: allocated: 0x74c0+0x478000
> > lib/relocator.c:1414: allocated 0x74c0/0x74c0
> >
> > Fixes: 3a5768645c05 ('First version of allocation from firmware')
> > Signed-off-by: Roger Pau Monné 
> 
> LGTM but I would like to hear Vladimir's opinion too.

Thanks Daniel, just a gentle ping to see if we can unblock this.

Regards, Roger.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel


Re: multiboot2 and module2 boot issues via GRUB2

2021-04-01 Thread Roger Pau Monné via Grub-devel
On Thu, Apr 01, 2021 at 09:31:07AM +0200, Jan Beulich wrote:
> On 01.04.2021 03:06, Roman Shaposhnik wrote:
> > And the obvious next question: is my EVE usecase esoteric enough that
> > I should just go ahead and do a custom GRUB patch or is there a more
> > general interest in this?
> 
> Not sure if it ought to be a grub patch - the issue could as well
> be dealt with in Xen, by concatenating modules to form a monolithic
> initrd.

I would rather have it done in the loader than Xen, mostly because
it's a Linux boot specific format, and hence I don't think Xen should
have any knowledge about it.

If it turns out to be impossible to implement on the loader side we
should consider doing it in Xen, but that's not my first option.

Thanks, Roger.

___
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel