[v5] x86_64: Improvements at compressed kernel stage

[PATCH v5 03/27] x86/boot: Set cr0 to known state in trampoline

Posted by Evgeniy Baskov 2 years, 6 months ago

Ensure WP bit to be set to prevent boot code from writing to
non-writable memory pages.

Tested-by: Mario Limonciello <mario.limonciello@amd.com>
Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
---
 arch/x86/boot/compressed/head_64.S | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
index 03c4328a88cb..01fa42d31648 100644
--- a/arch/x86/boot/compressed/head_64.S
+++ b/arch/x86/boot/compressed/head_64.S
@@ -660,9 +660,8 @@ SYM_CODE_START(trampoline_32bit_src)
 	pushl	$__KERNEL_CS
 	pushl	%eax
 
-	/* Enable paging again. */
-	movl	%cr0, %eax
-	btsl	$X86_CR0_PG_BIT, %eax
+	/* Enable paging and set CR0 to known state (this also sets WP flag) */
+	movl	$CR0_STATE, %eax
 	movl	%eax, %cr0
 
 	lret
-- 
2.39.2

Re: [PATCH v5 03/27] x86/boot: Set cr0 to known state in trampoline

Posted by Borislav Petkov 2 years, 5 months ago

On Tue, Mar 14, 2023 at 01:13:30PM +0300, Evgeniy Baskov wrote:
> Ensure WP bit to be set to prevent boot code from writing to
> non-writable memory pages.
> 
> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
> ---
>  arch/x86/boot/compressed/head_64.S | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> index 03c4328a88cb..01fa42d31648 100644
> --- a/arch/x86/boot/compressed/head_64.S
> +++ b/arch/x86/boot/compressed/head_64.S
> @@ -660,9 +660,8 @@ SYM_CODE_START(trampoline_32bit_src)
>  	pushl	$__KERNEL_CS
>  	pushl	%eax
>  
> -	/* Enable paging again. */
> -	movl	%cr0, %eax
> -	btsl	$X86_CR0_PG_BIT, %eax
> +	/* Enable paging and set CR0 to known state (this also sets WP flag) */
> +	movl	$CR0_STATE, %eax

This sets a lot more than WP. Why?

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette

Re: [PATCH v5 03/27] x86/boot: Set cr0 to known state in trampoline

Posted by Evgeniy Baskov 2 years, 5 months ago

On 2023-04-05 20:54, Borislav Petkov wrote:
> On Tue, Mar 14, 2023 at 01:13:30PM +0300, Evgeniy Baskov wrote:
>> Ensure WP bit to be set to prevent boot code from writing to
>> non-writable memory pages.
>> 
>> Tested-by: Mario Limonciello <mario.limonciello@amd.com>
>> Signed-off-by: Evgeniy Baskov <baskov@ispras.ru>
>> ---
>>  arch/x86/boot/compressed/head_64.S | 5 ++---
>>  1 file changed, 2 insertions(+), 3 deletions(-)
>> 
>> diff --git a/arch/x86/boot/compressed/head_64.S 
>> b/arch/x86/boot/compressed/head_64.S
>> index 03c4328a88cb..01fa42d31648 100644
>> --- a/arch/x86/boot/compressed/head_64.S
>> +++ b/arch/x86/boot/compressed/head_64.S
>> @@ -660,9 +660,8 @@ SYM_CODE_START(trampoline_32bit_src)
>>  	pushl	$__KERNEL_CS
>>  	pushl	%eax
>> 
>> -	/* Enable paging again. */
>> -	movl	%cr0, %eax
>> -	btsl	$X86_CR0_PG_BIT, %eax
>> +	/* Enable paging and set CR0 to known state (this also sets WP flag) 
>> */
>> +	movl	$CR0_STATE, %eax
> 
> This sets a lot more than WP. Why?

Because there are code paths where cr0 state is not initialized
(e.g. the EFISTUB code path) and it's better to know it exactly.
Although we don't actually care about MP, ET, NE and AM flags, but they
should be all supported, so the choice was arbitrary. Also they are 
already
initialized to this value on one code path -- when the kernel started 
its
execution via startup_32.

Thanks.