Re: [PATCH v2 00/23] x86_64: Improvements at compressed kernel stage

From: Limonciello, Mario
Date: Tue Nov 08 2022 - 13:17:38 EST


On 11/8/2022 01:01, Evgeniy Baskov wrote:
On 2022-11-04 21:21, Limonciello, Mario wrote:
On 10/25/2022 09:12, Evgeniy Baskov wrote:
...


Hi,

I was talking to Peter Jones recently about what was still missing for
NX support in the kernel and he pointed me at this series.

So I had a try with this series on top of:

ee6050c8af96 ("Merge tag 'ata-6.1-rc4' of
git://git.kernel.org/pub/scm/linux/kernel/git/dlemoal/libata")

Unfortunately I can't boot the system with this series applied.
This is not on a system that enforces NX pre-boot (but that was my
goal after I could prove booting on something that doesn't).
I didn't apply Peter's patch 6 you referenced in your cover letter,
but I don't expect that's the reason for the failure.

I get:

"Failed to allocate space for tmp_cmdline"

   -- System Halted

This is early enough [1] that I don't have anything else output to a
serial log from the kernel.

https://nam11.safelinks.protection.outlook.com/?url=https%3A%2F%2Fgithub.com%2Ftorvalds%2Flinux%2Fblob%2Fd4013bc4d49f6da8178a340348369bb9920225c9%2Farch%2Fx86%2Fboot%2Fcompressed%2Fkaslr.c%23L268&data=05%7C01%7Cmario.limonciello%40amd.com%7C9280e92e85bc4e2b52cd08dac15704a5%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C638034876740462327%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&sdata=oiCJUa8M3x%2FYCxVjJj98R7iU%2FzIj%2FQxdVOWbnqGWCNI%3D&reserved=0

Since this is only in the kaslr path, I tried to turn that off with
'nokaslr' on the kernel command line.

I then get a failure of:

"Out of memory while allocating zstd_dctx"

  -- System Halted

This kernel was booted from the following path:
-> Insyde BIOS
--> shim (from Fedora 36 repository)
---> GRUB (from Peter for Fedora 36 w/ some level NX support)
----> kernel binary (self-built)

The BIOS on this system doesn't validate NX, but also the shim binary
did not have the NX bit set in the PE header.

Your cover letter referenced CONFIG_EFI_STUB_EXTRACT_DIRECT but I
didn't find this option in the series.  I also tried both with
CONFIG_EFI_DXE_MEM_ATTRIBUTES=y or unset, same result.

Hi,

Thanks for your feedback!


Sure!

CONFIG_EFI_STUB_EXTRACT_DIRECT option was removed in v2 of the series
and direct extraction is unconditional now.

You are getting really weird errors, which unfortunately I am unable
to reproduce yet. I've tried booting with fedora's grub and the series
applied on top of ee6050c8af96, but it did boot successfully.

Well so I expect the unique difference is that I'm using Peter's GRUB that has some NX support landed. He has binaries for it here:

https://blog.uncooperative.org/~pjones/nx/repo/

*Theoretically* a BIOS that enforces NX should be able to boot a shim with
the NX compat bit set which should be able to boot that GRUB supporting NX which should be able to boot this series.


From the error messages it's some problem with malloc() implementation
of compressed kernel code. I suspect that malloc_ptr inside .bss is not
zeroed. This should not happen when booting via either non-UEFI
interface, or via UEFI (when kernel is properly loaded as PE image).
The problem, I think, arises when kernel is loaded as a blob, but EFI
handover protocol is used to start its execution. This is what grub
seems to be doing.

Can you please try booting with patches below applied on top?
If this fixes the problem, I'll include these changes in v3.

Yup, spot on. I can the kernel from Peter's NX enabled GRUB now with:
* 6.1-rc4
* Your existing 23 patch series
* this new patch

Thanks!!

Would you mind CC me when you submit v3? As I have an interest in seeing NX support I'd like to continue to follow along on the series.

Anything in the series you don't change in any material way from v2 please feel free to include:

Tested-by: Mario Limonciello <mario.limonciello@xxxxxxx>


Thanks,
Evgeniy Baskov
--
diff --git a/arch/x86/boot/compressed/head_32.S b/arch/x86/boot/compressed/head_32.S
index 9871cc8466fb..69811d9ab4ce 100644
--- a/arch/x86/boot/compressed/head_32.S
+++ b/arch/x86/boot/compressed/head_32.S
@@ -152,6 +152,14 @@ SYM_FUNC_END(startup_32)

 #ifdef CONFIG_EFI_STUB
 SYM_FUNC_START(efi32_stub_entry)
+    /* Clear BSS */
+    xorl    %eax, %eax
+    leal    _bss@GOTOFF(%ebx), %edi
+    leal    _ebss@GOTOFF(%ebx), %ecx
+    subl    %edi, %ecx
+    shrl    $2, %ecx
+    rep    stosl
+
     add    $0x4, %esp
     movl    8(%esp), %esi    /* save boot_params pointer */
     call    efi_main
diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 2bb0e6da08c0..384706d12354 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -553,8 +553,20 @@ SYM_CODE_END(startup_64)
 #ifdef CONFIG_EFI_STUB
     .org 0x390
 SYM_FUNC_START(efi64_stub_entry)
+    /* Preserve first parameter */
+    movq    %rdi, %r10
+
+    /* Clear BSS */
+    xorl    %eax, %eax
+    leaq    _bss(%rip), %rdi
+    leaq    _ebss(%rip), %rcx
+    subq    %rdi, %rcx
+    shrq    $3, %rcx
+    rep    stosq
+
     and    $~0xf, %rsp            /* realign the stack */
     movq    %rdx, %rbx            /* save boot_params pointer */
+    movq    %r10, %rdi
     call    efi_main

     cld
diff --git a/drivers/firmware/efi/libstub/x86-stub.c b/drivers/firmware/efi/libstub/x86-stub.c
index 95a69c37518e..1a2c52e77c6e 100644
--- a/drivers/firmware/efi/libstub/x86-stub.c
+++ b/drivers/firmware/efi/libstub/x86-stub.c
@@ -24,7 +24,7 @@

 const efi_system_table_t *efi_system_table;
 extern u32 image_offset;
-static efi_loaded_image_t *image = NULL;
+static efi_loaded_image_t *image __section(".data");

 extern char _head[], _ehead[];
 extern char _compressed[], _ecompressed[];