[PATCH 0/2] efi/arm64: execute the kernel in place if possible

[PATCH 0/2] efi/arm64: execute the kernel in place if possible

[Ard Biesheuvel]

Update the relocation logic in the EFI stub for arm64 so it runs the
kernel in place if it was loaded by firmware at an address which allows
doing so. Then, update the PE/COFF header metadata and increase the
section alignment to force the UEFI image loader to honour the minimal
alignment requirement imposed by the kernel proper.

Ard Biesheuvel (2):
  efi/libstub/arm64: avoid copying the kernel unnecessarily
  efi/arm64: increase the PE/COFF alignment so the kernel can run in
    place

 arch/arm64/kernel/efi-header.S            | 2 +-
 arch/arm64/kernel/image-vars.h            | 7 +++++++
 drivers/firmware/efi/libstub/arm64-stub.c | 9 +++++++++
 3 files changed, 17 insertions(+), 1 deletion(-)

-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] efi/libstub/arm64: avoid copying the kernel unnecessarily

[Ard Biesheuvel]

If the UEFI firmware has loaded the kernel at the right alignment, and
we are running a relocatable kernel, there is no point in copying the
kernel to a different place in memory, and instead, we can just run it
in place.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 drivers/firmware/efi/libstub/arm64-stub.c | 9 +++++++++
 1 file changed, 9 insertions(+)

diff --git a/drivers/firmware/efi/libstub/arm64-stub.c b/drivers/firmware/efi/libstub/arm64-stub.c
index 9254cd8ab2d3..d263f504dcf0 100644
--- a/drivers/firmware/efi/libstub/arm64-stub.c
+++ b/drivers/firmware/efi/libstub/arm64-stub.c
@@ -104,6 +104,15 @@ efi_status_t handle_kernel_image(unsigned long *image_addr,
 					  (u32)phys_seed);
 
 		*image_addr = *reserve_addr + offset;
+	} else if (IS_ENABLED(CONFIG_RELOCATABLE) &&
+		   IS_ALIGNED((u64)_text - TEXT_OFFSET, EFI_KIMG_ALIGN)) {
+		/*
+		 * If we are relocatable and were loaded at a suitable offset,
+		 * there is no need to do anything and we can just execute in
+		 * place.
+		 */
+		*image_addr = (u64)_text;
+		return EFI_SUCCESS;
 	} else {
 		/*
 		 * Else, try a straight allocation at the preferred offset.
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] efi/arm64: increase the PE/COFF alignment so the kernel can run in place

[Ard Biesheuvel]

Update the PE/COFF metadata so that the UEFI image loader will load the
kernel image at an offset that allows it to execute in place.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm64/kernel/efi-header.S | 2 +-
 arch/arm64/kernel/image-vars.h | 7 +++++++
 2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
index 914999ccaf8a..f9ee1c2a5fd6 100644
--- a/arch/arm64/kernel/efi-header.S
+++ b/arch/arm64/kernel/efi-header.S
@@ -32,7 +32,7 @@ optional_header:
 
 extra_header_fields:
 	.quad	0					// ImageBase
-	.long	SZ_4K					// SectionAlignment
+	.long	PECOFF_SECTION_ALIGNMENT		// SectionAlignment
 	.long	PECOFF_FILE_ALIGNMENT			// FileAlignment
 	.short	0					// MajorOperatingSystemVersion
 	.short	0					// MinorOperatingSystemVersion
diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
index be0a63ffed23..7a7fa3ba7b2f 100644
--- a/arch/arm64/kernel/image-vars.h
+++ b/arch/arm64/kernel/image-vars.h
@@ -15,6 +15,13 @@
 __efistub_kernel_size		= _edata - _text;
 __efistub_primary_entry_offset	= primary_entry - _text;
 
+#ifndef CONFIG_RELOCATABLE
+PECOFF_SECTION_ALIGNMENT = SZ_4K;
+#elif THREAD_ALIGN > SEGMENT_ALIGN
+PECOFF_SECTION_ALIGNMENT = THREAD_ALIGN;
+#else
+PECOFF_SECTION_ALIGNMENT = SEGMENT_ALIGN;
+#endif
 
 /*
  * The EFI stub has its own symbol namespace prefixed by __efistub_, to
-- 
2.17.1


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] efi/arm64: execute the kernel in place if possible

[Jonathan Cameron]

On Thu, 26 Mar 2020 17:59:03 +0100
Ard Biesheuvel <ardb@kernel.org> wrote:

> Update the relocation logic in the EFI stub for arm64 so it runs the
> kernel in place if it was loaded by firmware at an address which allows
> doing so. Then, update the PE/COFF header metadata and increase the
> section alignment to force the UEFI image loader to honour the minimal
> alignment requirement imposed by the kernel proper.
> 
> Ard Biesheuvel (2):
>   efi/libstub/arm64: avoid copying the kernel unnecessarily
>   efi/arm64: increase the PE/COFF alignment so the kernel can run in
>     place
> 
>  arch/arm64/kernel/efi-header.S            | 2 +-
>  arch/arm64/kernel/image-vars.h            | 7 +++++++
>  drivers/firmware/efi/libstub/arm64-stub.c | 9 +++++++++
>  3 files changed, 17 insertions(+), 1 deletion(-)
> 
Hi Ard.

Seems sensible to me so I decided to give it a quick test.
As things stand RELOCATABLE is only selectable by selecting
RANDOMIZE_BASE.   Probably want to be able to configure it
separately (needs some help text, or a specific option to
select CONFIG_RELOCATABLE).

Otherwise, superficially seems to work good for me doing a
boot via pxe/grub on a Kunpeng 920 with some prints added to
make sure it's taking the right paths.

Thanks,

Jonathan




_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] efi/arm64: execute the kernel in place if possible

[Ard Biesheuvel]

On Fri, 27 Mar 2020 at 14:19, Jonathan Cameron
<Jonathan.Cameron@huawei.com> wrote:
>
> On Thu, 26 Mar 2020 17:59:03 +0100
> Ard Biesheuvel <ardb@kernel.org> wrote:
>
> > Update the relocation logic in the EFI stub for arm64 so it runs the
> > kernel in place if it was loaded by firmware at an address which allows
> > doing so. Then, update the PE/COFF header metadata and increase the
> > section alignment to force the UEFI image loader to honour the minimal
> > alignment requirement imposed by the kernel proper.
> >
> > Ard Biesheuvel (2):
> >   efi/libstub/arm64: avoid copying the kernel unnecessarily
> >   efi/arm64: increase the PE/COFF alignment so the kernel can run in
> >     place
> >
> >  arch/arm64/kernel/efi-header.S            | 2 +-
> >  arch/arm64/kernel/image-vars.h            | 7 +++++++
> >  drivers/firmware/efi/libstub/arm64-stub.c | 9 +++++++++
> >  3 files changed, 17 insertions(+), 1 deletion(-)
> >
> Hi Ard.
>
> Seems sensible to me so I decided to give it a quick test.
> As things stand RELOCATABLE is only selectable by selecting
> RANDOMIZE_BASE.   Probably want to be able to configure it
> separately (needs some help text, or a specific option to
> select CONFIG_RELOCATABLE).
>

The idea is really that KASLR kernels that were booted without a seed
(or with 'nokaslr' on the command line) take this path. But in fact, I
realized that the same logic applies to non-relocatable kernels, just
with an alignment of 2 MB rather than 64 KB or 128 KB. So I will be
sending a v2 that looks slightly different. (Another thing that I
realized is that /if/ the KASLR path fails for any reason, we could
still take this path instead of doing the allocation)

> Otherwise, superficially seems to work good for me doing a
> boot via pxe/grub on a Kunpeng 920 with some prints added to
> make sure it's taking the right paths.
>

Excellent! Thanks for testing, and I will be cc'ing you on v2.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] efi/arm64: increase the PE/COFF alignment so the kernel can run in place

[Arvind Sankar]

On Thu, Mar 26, 2020 at 05:59:05PM +0100, Ard Biesheuvel wrote:
> Update the PE/COFF metadata so that the UEFI image loader will load the
> kernel image at an offset that allows it to execute in place.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> ---
>  arch/arm64/kernel/efi-header.S | 2 +-
>  arch/arm64/kernel/image-vars.h | 7 +++++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
> index 914999ccaf8a..f9ee1c2a5fd6 100644
> --- a/arch/arm64/kernel/efi-header.S
> +++ b/arch/arm64/kernel/efi-header.S
> @@ -32,7 +32,7 @@ optional_header:
>  
>  extra_header_fields:
>  	.quad	0					// ImageBase
> -	.long	SZ_4K					// SectionAlignment
> +	.long	PECOFF_SECTION_ALIGNMENT		// SectionAlignment
>  	.long	PECOFF_FILE_ALIGNMENT			// FileAlignment
>  	.short	0					// MajorOperatingSystemVersion
>  	.short	0					// MinorOperatingSystemVersion
> diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> index be0a63ffed23..7a7fa3ba7b2f 100644
> --- a/arch/arm64/kernel/image-vars.h
> +++ b/arch/arm64/kernel/image-vars.h
> @@ -15,6 +15,13 @@
>  __efistub_kernel_size		= _edata - _text;
>  __efistub_primary_entry_offset	= primary_entry - _text;
>  
> +#ifndef CONFIG_RELOCATABLE
> +PECOFF_SECTION_ALIGNMENT = SZ_4K;
> +#elif THREAD_ALIGN > SEGMENT_ALIGN
> +PECOFF_SECTION_ALIGNMENT = THREAD_ALIGN;
> +#else
> +PECOFF_SECTION_ALIGNMENT = SEGMENT_ALIGN;
> +#endif
>  
>  /*
>   * The EFI stub has its own symbol namespace prefixed by __efistub_, to
> -- 
> 2.17.1
> 

The section virtual addresses and (possibly) size of image need to be
updated to be a multiple of PECOFF_SECTION_ALIGNMENT, no?

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] efi/arm64: increase the PE/COFF alignment so the kernel can run in place

[Ard Biesheuvel]

Hi Arvind,

Thanks for taking a look.

On Sat, 28 Mar 2020 at 15:06, Arvind Sankar <nivedita@alum.mit.edu> wrote:
>
> On Thu, Mar 26, 2020 at 05:59:05PM +0100, Ard Biesheuvel wrote:
> > Update the PE/COFF metadata so that the UEFI image loader will load the
> > kernel image at an offset that allows it to execute in place.
> >
> > Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
> > ---
> >  arch/arm64/kernel/efi-header.S | 2 +-
> >  arch/arm64/kernel/image-vars.h | 7 +++++++
> >  2 files changed, 8 insertions(+), 1 deletion(-)
> >
> > diff --git a/arch/arm64/kernel/efi-header.S b/arch/arm64/kernel/efi-header.S
> > index 914999ccaf8a..f9ee1c2a5fd6 100644
> > --- a/arch/arm64/kernel/efi-header.S
> > +++ b/arch/arm64/kernel/efi-header.S
> > @@ -32,7 +32,7 @@ optional_header:
> >
> >  extra_header_fields:
> >       .quad   0                                       // ImageBase
> > -     .long   SZ_4K                                   // SectionAlignment
> > +     .long   PECOFF_SECTION_ALIGNMENT                // SectionAlignment
> >       .long   PECOFF_FILE_ALIGNMENT                   // FileAlignment
> >       .short  0                                       // MajorOperatingSystemVersion
> >       .short  0                                       // MinorOperatingSystemVersion
> > diff --git a/arch/arm64/kernel/image-vars.h b/arch/arm64/kernel/image-vars.h
> > index be0a63ffed23..7a7fa3ba7b2f 100644
> > --- a/arch/arm64/kernel/image-vars.h
> > +++ b/arch/arm64/kernel/image-vars.h
> > @@ -15,6 +15,13 @@
> >  __efistub_kernel_size                = _edata - _text;
> >  __efistub_primary_entry_offset       = primary_entry - _text;
> >
> > +#ifndef CONFIG_RELOCATABLE
> > +PECOFF_SECTION_ALIGNMENT = SZ_4K;
> > +#elif THREAD_ALIGN > SEGMENT_ALIGN
> > +PECOFF_SECTION_ALIGNMENT = THREAD_ALIGN;
> > +#else
> > +PECOFF_SECTION_ALIGNMENT = SEGMENT_ALIGN;
> > +#endif
> >
> >  /*
> >   * The EFI stub has its own symbol namespace prefixed by __efistub_, to
> > --
> > 2.17.1
> >
>
> The section virtual addresses and (possibly) size of image need to be
> updated to be a multiple of PECOFF_SECTION_ALIGNMENT, no?

Indeed. I spotted that after sending this patch - both _end and
_initdata_begin need to be aligned to this value. I also noticed that
it is unclear whether values over 64 KB are permitted: the PE/COFF
spec mentions that 64 KB is the max for FileAlignment, and that
SectionAlignment should be larger or equal to that.

So I think it would be better to set this value to 64 KB
unconditionally, and round up the sections to 64 KB. This means 64k
pagesize kernels with vmap'ed stack have a 50% chance of ending up at
an offset that requires moving the image, but this is still an
improvement over doing it all the time.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel