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:   Thu, 20 Oct 2022 14:23:16 +0300
From:   Evgeniy Baskov <baskov@ispras.ru>
To:     Ard Biesheuvel <ardb@kernel.org>
Cc:     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 03/16] x86/boot: Set cr0 to known state in trampoline
In-Reply-To: <CAMj1kXHUiBO0d926eWTtWZp_hvwHsVLzzn+fdS1EYoXHxpH+iA@mail.gmail.com>
References: <cover.1662459668.git.baskov@ispras.ru>
 <a155c20eec6343d34b3bdf40c024ce8b35a90e02.1662459668.git.baskov@ispras.ru>
 <CAMj1kXHUiBO0d926eWTtWZp_hvwHsVLzzn+fdS1EYoXHxpH+iA@mail.gmail.com>
User-Agent: Roundcube Webmail/1.4.4
Message-ID: <3591ec1f2195095effd08e11fe45986b@ispras.ru>
Content-Type: text/plain; charset=US-ASCII;
 format=flowed
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2022-10-19 10:06, Ard Biesheuvel wrote:
> On Tue, 6 Sept 2022 at 12:41, Evgeniy Baskov <baskov@ispras.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@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 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
>>