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 PHC | |
Open Source and information security mailing list archives
| ||
|
Date: Sat, 11 Mar 2023 17:37:45 +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>, 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 2023-03-10 17:56, Ard Biesheuvel wrote: > 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? Yes, probably it is just a leftover of the approach I used before. I'll remove that. > >> + /* >> + * 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? > Will fix. >> + */ >> + 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