| 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: <fbb6349f-4195-4d83-8d51-99bb91ea4fe0@oracle.com>
Date: Fri, 7 Mar 2025 11:33:06 -0800
From: ross.philipson@...cle.com
To: Jarkko Sakkinen <jarkko@...nel.org>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org,
linux-integrity@...r.kernel.org, linux-doc@...r.kernel.org,
linux-crypto@...r.kernel.org, kexec@...ts.infradead.org,
linux-efi@...r.kernel.org, iommu@...ts.linux-foundation.org,
dpsmith@...rtussolutions.com, tglx@...utronix.de, mingo@...hat.com,
bp@...en8.de, hpa@...or.com, dave.hansen@...ux.intel.com,
ardb@...nel.org, mjg59@...f.ucam.org,
James.Bottomley@...senpartnership.com, peterhuewe@....de, jgg@...pe.ca,
luto@...capital.net, nivedita@...m.mit.edu,
herbert@...dor.apana.org.au, davem@...emloft.net, corbet@....net,
ebiederm@...ssion.com, dwmw2@...radead.org, baolu.lu@...ux.intel.com,
kanth.ghatraju@...cle.com, andrew.cooper3@...rix.com,
trenchboot-devel@...glegroups.com
Subject: Re: [PATCH v12 08/19] x86/boot: Place TXT MLE header in the
kernel_info section
On 3/6/25 10:51 PM, Jarkko Sakkinen wrote:
> On Thu, Dec 19, 2024 at 11:42:05AM -0800, Ross Philipson wrote:
>> The MLE (measured launch environment) header must be locatable by the
>> boot loader and TXT must be setup to do a launch with this header's
>
> (cutting the hairs) nit: /TXT/Intel TXT/
>
>> location. While the offset to the kernel_info structure does not need
>> to be at a fixed offset, the offsets in the header must be relative
>> offsets from the start of the setup kernel. The support in the linker
>> file achieves this.
>
> This is too obfuscated and also sort of misses the action taken by
> the patch.
>
> I presume the goal here is to add relative offset to the MLE header?
> Please state that explicitly.
>
> Like for any possible kernel patch:
>
> 1. Come out clean 110% transparent.
> 2. Full exposure what you're doing.
>
> ;-)
>
> That's the fastest possible path to actual results.
>
>>
>> Signed-off-by: Ross Philipson <ross.philipson@...cle.com>
>> Suggested-by: Ard Biesheuvel <ardb@...nel.org>
>> Reviewed-by: Ard Biesheuvel <ardb@...nel.org>
>> ---
>> arch/x86/boot/compressed/kernel_info.S | 50 +++++++++++++++++++++++---
>> arch/x86/boot/compressed/vmlinux.lds.S | 7 ++++
>> 2 files changed, 53 insertions(+), 4 deletions(-)
>>
>> diff --git a/arch/x86/boot/compressed/kernel_info.S b/arch/x86/boot/compressed/kernel_info.S
>> index f818ee8fba38..a0604a0d1756 100644
>> --- a/arch/x86/boot/compressed/kernel_info.S
>> +++ b/arch/x86/boot/compressed/kernel_info.S
>> @@ -1,12 +1,20 @@
>> /* SPDX-License-Identifier: GPL-2.0 */
>>
>> +#include <linux/linkage.h>
>> #include <asm/bootparam.h>
>>
>> - .section ".rodata.kernel_info", "a"
>> +/*
>> + * The kernel_info structure is not placed at a fixed offest in the
>> + * kernel image. So this macro and the support in the linker file
>> + * allow the relative offsets for the MLE header within the kernel
>> + * image to be configured at build time.
>> + */
>> +#define roffset(X) ((X) - kernel_info)
>>
>> - .global kernel_info
>> + .section ".rodata.kernel_info", "a"
>>
>> -kernel_info:
>> + .balign 16
>> +SYM_DATA_START(kernel_info)
>> /* Header, Linux top (structure). */
>> .ascii "LToP"
>> /* Size. */
>> @@ -17,6 +25,40 @@ kernel_info:
>> /* Maximal allowed type for setup_data and setup_indirect structs. */
>> .long SETUP_TYPE_MAX
>>
>> + /* Offset to the MLE header structure */
>> +#if IS_ENABLED(CONFIG_SECURE_LAUNCH)
>> + .long roffset(mle_header_offset)
>> +#else
>> + .long 0
>> +#endif
>> +
>> kernel_info_var_len_data:
>> /* Empty for time being... */
>> -kernel_info_end:
>> +SYM_DATA_END_LABEL(kernel_info, SYM_L_LOCAL, kernel_info_end)
>> +
>> +#if IS_ENABLED(CONFIG_SECURE_LAUNCH)
>> + /*
>> + * The MLE Header per the TXT Specification, section 2.1
>> + * MLE capabilities, see table 4. Capabilities set:
>> + * bit 0: Support for GETSEC[WAKEUP] for RLP wakeup
>> + * bit 1: Support for RLP wakeup using MONITOR address
>> + * bit 2: The ECX register will contain the pointer to the MLE page table
>> + * bit 5: TPM 1.2 family: Details/authorities PCR usage support
>> + * bit 9: Supported format of TPM 2.0 event log - TCG compliant
>> + */
>> +SYM_DATA_START(mle_header)
>> + .long 0x9082ac5a /* UUID0 */
>> + .long 0x74a7476f /* UUID1 */
>> + .long 0xa2555c0f /* UUID2 */
>> + .long 0x42b651cb /* UUID3 */
>> + .long 0x00000034 /* MLE header size */
>> + .long 0x00020002 /* MLE version 2.2 */
>> + .long roffset(sl_stub_entry_offset) /* Linear entry point of MLE (virt. address) */
>> + .long 0x00000000 /* First valid page of MLE */
>> + .long 0x00000000 /* Offset within binary of first byte of MLE */
>> + .long roffset(_edata_offset) /* Offset within binary of last byte + 1 of MLE */
>> + .long 0x00000227 /* Bit vector of MLE-supported capabilities */
>> + .long 0x00000000 /* Starting linear address of command line (unused) */
>> + .long 0x00000000 /* Ending linear address of command line (unused) */
>
> Nit: I'd consider aligning these to few tab offsets after even tho it
> might cause checkpatch complain (which is fine when there are legitimite
> reasons to do so).
>
> Would be easier to read.
>
>> +SYM_DATA_END(mle_header)
>> +#endif
>> diff --git a/arch/x86/boot/compressed/vmlinux.lds.S b/arch/x86/boot/compressed/vmlinux.lds.S
>> index 083ec6d7722a..f82184801462 100644
>> --- a/arch/x86/boot/compressed/vmlinux.lds.S
>> +++ b/arch/x86/boot/compressed/vmlinux.lds.S
>> @@ -118,3 +118,10 @@ SECTIONS
>> }
>> ASSERT(SIZEOF(.rela.dyn) == 0, "Unexpected run-time relocations (.rela) detected!")
>> }
>> +
>> +#ifdef CONFIG_SECURE_LAUNCH
>> +PROVIDE(kernel_info_offset = ABSOLUTE(kernel_info - startup_32));
>> +PROVIDE(mle_header_offset = kernel_info_offset + ABSOLUTE(mle_header - startup_32));
>> +PROVIDE(sl_stub_entry_offset = kernel_info_offset + ABSOLUTE(sl_stub_entry - startup_32));
>> +PROVIDE(_edata_offset = kernel_info_offset + ABSOLUTE(_edata - startup_32));
>
> I'd enumerate these one by one in the commit message. I.e. what is added
> explicitly.
Everything you say here makes sense, we can incorporate these changes.
Thanks
Ross
>
>> +#endif
>> --
>> 2.39.3
>>
>
> BR, Jarkko
Powered by blists - more mailing lists