| 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
| ||
|
Message-ID: <CAMj1kXGwaX8nGJdRM3==thO=KfXb3UwbF2jJhrqiZK-RcM+QLw@mail.gmail.com>
Date: Fri, 10 Mar 2023 15:56:20 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Evgeniy Baskov <baskov@...ras.ru>
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>,
Peter Jones <pjones@...hat.com>,
"Limonciello, Mario" <mario.limonciello@....com>,
joeyli <jlee@...e.com>, 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 v4 13/26] x86/boot: Split trampoline and pt init code
On Thu, 15 Dec 2022 at 13:40, Evgeniy Baskov <baskov@...ras.ru> wrote:
>
> When allocating trampoline from libstub trampoline allocation is
> performed separately, so it needs to be skipped.
>
> Split trampoline initialization and allocation code into two
> functions to make them invokable separately.
>
> Tested-by: Mario Limonciello <mario.limonciello@....com>
> Tested-by: Peter Jones <pjones@...hat.com>
> Signed-off-by: Evgeniy Baskov <baskov@...ras.ru>
> ---
> arch/x86/boot/compressed/pgtable_64.c | 73 +++++++++++++++++----------
> 1 file changed, 46 insertions(+), 27 deletions(-)
>
> diff --git a/arch/x86/boot/compressed/pgtable_64.c b/arch/x86/boot/compressed/pgtable_64.c
> index c7cf5a1059a8..1f7169248612 100644
> --- a/arch/x86/boot/compressed/pgtable_64.c
> +++ b/arch/x86/boot/compressed/pgtable_64.c
> @@ -106,12 +106,8 @@ static unsigned long find_trampoline_placement(void)
> return bios_start - TRAMPOLINE_32BIT_SIZE;
> }
>
> -struct paging_config paging_prepare(void *rmode)
> +bool trampoline_pgtable_init(struct boot_params *boot_params)
> {
> - struct paging_config paging_config = {};
> -
> - /* Initialize boot_params. Required for cmdline_find_option_bool(). */
> - boot_params = rmode;
>
> /*
> * Check if LA57 is desired and supported.
> @@ -125,26 +121,10 @@ struct paging_config paging_prepare(void *rmode)
> *
> * That's substitute for boot_cpu_has() in early boot code.
> */
> - if (IS_ENABLED(CONFIG_X86_5LEVEL) &&
> - !cmdline_find_option_bool("no5lvl") &&
> - native_cpuid_eax(0) >= 7 &&
> - (native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31)))) {
> - paging_config.l5_required = 1;
> - }
> -
> - paging_config.trampoline_start = find_trampoline_placement();
> -
> - trampoline_32bit = (unsigned long *)paging_config.trampoline_start;
> -
> - /* Preserve trampoline memory */
> - memcpy(trampoline_save, trampoline_32bit, TRAMPOLINE_32BIT_SIZE);
> -
> - /* Clear trampoline memory first */
> - memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
> -
> - /* Copy trampoline code in place */
> - memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / sizeof(unsigned long),
> - &trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
> + bool l5_required = IS_ENABLED(CONFIG_X86_5LEVEL) &&
> + !cmdline_find_option_bool("no5lvl") &&
> + native_cpuid_eax(0) >= 7 &&
> + (native_cpuid_ecx(7) & (1 << (X86_FEATURE_LA57 & 31)));
>
> /*
> * The code below prepares page table in trampoline memory.
> @@ -160,10 +140,10 @@ struct paging_config paging_prepare(void *rmode)
> * We are not going to use the page table in trampoline memory if we
> * are already in the desired paging mode.
> */
> - if (paging_config.l5_required == !!(native_read_cr4() & X86_CR4_LA57))
> + if (l5_required == !!(native_read_cr4() & X86_CR4_LA57))
> goto out;
>
> - if (paging_config.l5_required) {
> + if (l5_required) {
> /*
> * For 4- to 5-level paging transition, set up current CR3 as
> * the first and the only entry in a new top-level page table.
> @@ -185,6 +165,45 @@ struct paging_config paging_prepare(void *rmode)
> (void *)src, PAGE_SIZE);
> }
>
> +out:
> + return l5_required;
> +}
> +
> +struct paging_config paging_prepare(void *rmode)
> +{
> + struct paging_config paging_config = {};
> + bool early_trampoline_alloc = 0;
false
> +
> + /* Initialize boot_params. Required for cmdline_find_option_bool(). */
> + boot_params = rmode;
> +
> + /*
> + * We only need to find trampoline placement, if we have
> + * not already done it from libstub.
> + */
> +
> + paging_config.trampoline_start = find_trampoline_placement();
> + trampoline_32bit = (unsigned long *)paging_config.trampoline_start;
> + early_trampoline_alloc = 0;
> +
false again
And it never becomes true, nor is it used anywhere else. Can we get rid of it?
> + /*
> + * Preserve trampoline memory.
> + * When trampoline is located in memory
> + * owned by us, i.e. allocated in EFISTUB,
> + * we don't care about previous contents
> + * of this memory so copying can also be skipped.
Can you please reflow comments so they takes up fewer lines?
> + */
> + memcpy(trampoline_save, trampoline_32bit, TRAMPOLINE_32BIT_SIZE);
> +
> + /* Clear trampoline memory first */
> + memset(trampoline_32bit, 0, TRAMPOLINE_32BIT_SIZE);
> +
> + /* Copy trampoline code in place */
> + memcpy(trampoline_32bit + TRAMPOLINE_32BIT_CODE_OFFSET / sizeof(unsigned long),
> + &trampoline_32bit_src, TRAMPOLINE_32BIT_CODE_SIZE);
> +
> + paging_config.l5_required = trampoline_pgtable_init(boot_params);
> +
> out:
> return paging_config;
> }
> --
> 2.37.4
>
Powered by blists - more mailing lists