linux-kernel - Re: [PATCH 03/16] x86/boot: Set cr0 to known state in trampoline

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <3591ec1f2195095effd08e11fe45986b@ispras.ru>
Date:   Thu, 20 Oct 2022 14:23:16 +0300
From:   Evgeniy Baskov <baskov@...ras.ru>
To:     Ard Biesheuvel <ardb@...nel.org>
Cc:     Borislav Petkov <bp@...en8.de>, Andy Lutomirski <luto@...nel.org>,
        Dave Hansen <dave.hansen@...ux.intel.com>,
        Ingo Molnar <mingo@...hat.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Alexey Khoroshilov <khoroshilov@...ras.ru>,
        lvc-project@...uxtesting.org, x86@...nel.org,
        linux-efi@...r.kernel.org, linux-kernel@...r.kernel.org,
        linux-hardening@...r.kernel.org
Subject: Re: [PATCH 03/16] x86/boot: Set cr0 to known state in trampoline

On 2022-10-19 10:06, Ard Biesheuvel wrote:
> On Tue, 6 Sept 2022 at 12:41, Evgeniy Baskov <baskov@...ras.ru> wrote:
>> 
>> Ensure WP bit to be set to prevent boot code from writing to
>> non-writable memory pages.
>> 
>> Signed-off-by: Evgeniy Baskov <baskov@...ras.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 d33f060900d2..5273367283b7 100644
>> --- a/arch/x86/boot/compressed/head_64.S
>> +++ b/arch/x86/boot/compressed/head_64.S
>> @@ -619,9 +619,8 @@ SYM_CODE_START(trampoline_32bit_src)
>>         /* Set up new stack */
>>         leal    TRAMPOLINE_32BIT_STACK_END(%ecx), %esp
>> 
>> -       /* Disable paging */
>> -       movl    %cr0, %eax
>> -       btrl    $X86_CR0_PG_BIT, %eax
> 
> Why do we no longer care about CR0's prior value?

I think we don't need to preserve any of those flags
(we nether use floating point instructions nor call EFI functions
with this cr0 value) and it's better to set cr0 to the well-known
state. CR0 is also being set to the constant value while switching
from protected to long mode, so it is already done in one of the
code paths.

If I am missing something, let me know,
I will change it to only set WP and clear PG.

> 
>> +       /* Disable paging and setup CR0 */
>> +       movl    $(CR0_STATE & ~X86_CR0_PG), %eax
>>         movl    %eax, %cr0
>> 
>>         /* Check what paging mode we want to be in after the 
>> trampoline */
>> --
>> 2.35.1
>>