Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
MIME-Version: 1.0
Date:   Tue, 08 Nov 2022 10:01:07 +0300
From:   Evgeniy Baskov <baskov@ispras.ru>
To:     "Limonciello, Mario" <mario.limonciello@amd.com>
Cc:     Ard Biesheuvel <ardb@kernel.org>, Peter Jones <pjones@redhat.com>,
        Borislav Petkov <bp@alien8.de>,
        Andy Lutomirski <luto@kernel.org>,
        Dave Hansen <dave.hansen@linux.intel.com>,
        Ingo Molnar <mingo@redhat.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Alexey Khoroshilov <khoroshilov@ispras.ru>,
        lvc-project@linuxtesting.org, x86@kernel.org,
        linux-efi@vger.kernel.org, linux-kernel@vger.kernel.org,
        linux-hardening@vger.kernel.org
Subject: Re: [PATCH v2 00/23] x86_64: Improvements at compressed kernel stage
In-Reply-To: <e2beec0b-22d3-91bd-c57c-8c8ad2137406@amd.com>
References: <cover.1666705333.git.baskov@ispras.ru>
 <e2beec0b-22d3-91bd-c57c-8c8ad2137406@amd.com>
User-Agent: Roundcube Webmail/1.4.4
Message-ID: <d97fdff486959dbf841c92b3cb644740@ispras.ru>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

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://github.com/torvalds/linux/blob/d4013bc4d49f6da8178a340348369bb9920225c9/arch/x86/boot/compressed/kaslr.c#L268
> 
> 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!

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.

 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.

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[];