| Message ID | 20230504145023.835096-1-ross.philipson@oracle.com |
|---|---|
| Headers | show |
| Series | x86: Trenchboot secure dynamic launch Linux kernel support | expand |
On Thu, May 04, 2023 at 02:50:09PM +0000, Ross Philipson wrote: > This patchset provides detailed documentation of DRTM, the approach used for > adding the capbility, and relevant API/ABI documentation. In addition to the > documentation the patch set introduces Intel TXT support as the first platform > for Linux Secure Launch. I'd like to apply this series, but on what commit it is based on? I don't see any branch containing this series version in trenchboot tree [1]. Thanks. [1]: https://github.com/TrenchBoot/linux
On 5/5/23 04:39, Bagas Sanjaya wrote: > On Thu, May 04, 2023 at 02:50:09PM +0000, Ross Philipson wrote: >> This patchset provides detailed documentation of DRTM, the approach used for >> adding the capbility, and relevant API/ABI documentation. In addition to the >> documentation the patch set introduces Intel TXT support as the first platform >> for Linux Secure Launch. > > I'd like to apply this series, but on what commit it is based on? I > don't see any branch containing this series version in trenchboot tree > [1]. Sorry about that. In the future I will include a base-commit field. It is based off of torvolds/master as of 5/1/2023. The branch where the patches came from is now pushed to the TrenchBoot repository here: https://github.com/TrenchBoot/linux/tree/linux-sl-master-5-1-23-v6 Thanks Ross > > Thanks. > > [1]: https://github.com/TrenchBoot/linux >
On Thu, May 04, 2023 at 02:50:15PM +0000, Ross Philipson wrote: > From: "Daniel P. Smith" <dpsmith@apertussolutions.com> > > The SHA algorithms are necessary to measure configuration information into > the TPM as early as possible before using the values. This implementation > uses the established approach of #including the SHA libraries directly in > the code since the compressed kernel is not uncompressed at this point. > > The SHA code here has its origins in the code from the main kernel: > > commit c4d5b9ffa31f ("crypto: sha1 - implement base layer for SHA-1") > > That code could not be pulled directly into the setup portion of the > compressed kernel because of other dependencies it pulls in. The result > is this is a modified copy of that code that still leverages the core > SHA algorithms. > > Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> > Signed-off-by: Ross Philipson <ross.philipson@oracle.com> > --- > arch/x86/boot/compressed/Makefile | 2 + > arch/x86/boot/compressed/early_sha1.c | 97 +++++++++++++++++++++++++++++++++ > arch/x86/boot/compressed/early_sha1.h | 17 ++++++ > arch/x86/boot/compressed/early_sha256.c | 7 +++ > lib/crypto/sha1.c | 4 ++ > lib/crypto/sha256.c | 8 +++ > 6 files changed, 135 insertions(+) > create mode 100644 arch/x86/boot/compressed/early_sha1.c > create mode 100644 arch/x86/boot/compressed/early_sha1.h > create mode 100644 arch/x86/boot/compressed/early_sha256.c > > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile > index 6b6cfe6..1d327d4 100644 > --- a/arch/x86/boot/compressed/Makefile > +++ b/arch/x86/boot/compressed/Makefile > @@ -112,6 +112,8 @@ vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o > vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o > vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a > > +vmlinux-objs-$(CONFIG_SECURE_LAUNCH) += $(obj)/early_sha1.o $(obj)/early_sha256.o > + > $(obj)/vmlinux: $(vmlinux-objs-y) FORCE > $(call if_changed,ld) > > diff --git a/arch/x86/boot/compressed/early_sha1.c b/arch/x86/boot/compressed/early_sha1.c > new file mode 100644 > index 0000000..524ec23 > --- /dev/null > +++ b/arch/x86/boot/compressed/early_sha1.c > @@ -0,0 +1,97 @@ > +// SPDX-License-Identifier: GPL-2.0 > +/* > + * Copyright (c) 2022 Apertus Solutions, LLC. > + */ > + > +#include <linux/init.h> > +#include <linux/linkage.h> > +#include <linux/string.h> > +#include <asm/boot.h> > +#include <asm/unaligned.h> > + > +#include "early_sha1.h" > + > +#define SHA1_DISABLE_EXPORT Hi Ross, I could be mistaken, but it seems to me that *_DISABLE_EXPORT is a mechanism of this patch to disable exporting symbols (use of EXPORT_SYMBOL), at compile time. If so, this doesn't strike me as something that should be part of upstream kernel code. Could you consider dropping this part of the patch? ...
On Thu, May 04, 2023 at 02:50:16PM +0000, Ross Philipson wrote: > The Secure Launch (SL) stub provides the entry point for Intel TXT (and > later AMD SKINIT) to vector to during the late launch. The symbol > sl_stub_entry is that entry point and its offset into the kernel is > conveyed to the launching code using the MLE (Measured Launch > Environment) header in the structure named mle_header. The offset of the > MLE header is set in the kernel_info. The routine sl_stub contains the > very early late launch setup code responsible for setting up the basic > environment to allow the normal kernel startup_32 code to proceed. It is > also responsible for properly waking and handling the APs on Intel > platforms. The routine sl_main which runs after entering 64b mode is > responsible for measuring configuration and module information before > it is used like the boot params, the kernel command line, the TXT heap, > an external initramfs, etc. > > Signed-off-by: Ross Philipson <ross.philipson@oracle.com> ... > diff --git a/arch/x86/boot/compressed/sl_main.c b/arch/x86/boot/compressed/sl_main.c ... > +static void *evtlog_base; > +static u32 evtlog_size; > +static struct txt_heap_event_log_pointer2_1_element *log20_elem; > +static u32 tpm_log_ver = SL_TPM12_LOG; > +struct tcg_efi_specid_event_algs tpm_algs[SL_TPM20_MAX_ALGS] = {0}; tpm_algs seems to only be used in this file. Should it be static? > + > +extern u32 sl_cpu_type; > +extern u32 sl_mle_start; > +extern struct boot_params *boot_params; > + > +static u64 sl_txt_read(u32 reg) Perhaps reg should have an __iomem annotation. > +{ > + return readq((void *)(u64)(TXT_PRIV_CONFIG_REGS_BASE + reg)); > +} > + > +static void sl_txt_write(u32 reg, u64 val) Likewise here. ... > +static void sl_check_pmr_coverage(void *base, u32 size, bool allow_hi) > +{ > + struct txt_os_sinit_data *os_sinit_data; > + void *end = base + size; > + void *txt_heap; > + > + if (!(sl_cpu_type & SL_CPU_INTEL)) > + return; > + > + txt_heap = (void *)sl_txt_read(TXT_CR_HEAP_BASE); > + os_sinit_data = txt_os_sinit_data_start(txt_heap); > + > + if ((end >= (void *)0x100000000ULL) && > + (base < (void *)0x100000000ULL)) > + sl_txt_reset(SL_ERROR_REGION_STRADDLE_4GB); > + > + /* > + * Note that the late stub code validates that the hi PMR covers > + * all memory above 4G. At this point the code can only check that > + * regions are within the hi PMR but that is sufficient. > + */ > + if ((end > (void *)0x100000000ULL) && > + (base >= (void *)0x100000000ULL)) { > + if (allow_hi) { > + if (end >= (void *)(os_sinit_data->vtd_pmr_hi_base + > + os_sinit_data->vtd_pmr_hi_size)) > + sl_txt_reset(SL_ERROR_BUFFER_BEYOND_PMR); > + } else > + sl_txt_reset(SL_ERROR_REGION_ABOVE_4GB); nit: if any arm of a condition has '{}' then all arms should have them. So: } else { sl_txt_reset(SL_ERROR_REGION_ABOVE_4GB); } Also elsewhere in this patch. > + } > + > + if (end >= (void *)os_sinit_data->vtd_pmr_lo_size) > + sl_txt_reset(SL_ERROR_BUFFER_BEYOND_PMR); > +} > + > +/* > + * Some MSRs are modified by the pre-launch code including the MTRRs. > + * The early MLE code has to restore these values. This code validates > + * the values after they are measured. > + */ > +static void sl_txt_validate_msrs(struct txt_os_mle_data *os_mle_data) > +{ > + struct slr_txt_mtrr_state *saved_bsp_mtrrs; > + u64 mtrr_caps, mtrr_def_type, mtrr_var; > + struct slr_entry_intel_info *txt_info; > + u64 misc_en_msr; > + u32 vcnt, i; > + > + txt_info = (struct slr_entry_intel_info *)os_mle_data->txt_info; > + saved_bsp_mtrrs = &(txt_info->saved_bsp_mtrrs); nit: unnecessary parentheses ... > +static void sl_validate_event_log_buffer(void) > +{ > + struct txt_os_sinit_data *os_sinit_data; > + void *txt_heap, *txt_end; > + void *mle_base, *mle_end; > + void *evtlog_end; > + > + if ((u64)evtlog_size > (LLONG_MAX - (u64)evtlog_base)) > + sl_txt_reset(SL_ERROR_INTEGER_OVERFLOW); > + evtlog_end = evtlog_base + evtlog_size; > + > + txt_heap = (void *)sl_txt_read(TXT_CR_HEAP_BASE); > + txt_end = txt_heap + sl_txt_read(TXT_CR_HEAP_SIZE); > + os_sinit_data = txt_os_sinit_data_start(txt_heap); > + > + mle_base = (void *)(u64)sl_mle_start; > + mle_end = mle_base + os_sinit_data->mle_size; > + > + /* > + * This check is to ensure the event log buffer does not overlap with > + * the MLE image. > + */ > + if ((evtlog_base >= mle_end) && > + (evtlog_end > mle_end)) > + goto pmr_check; /* above */ Ditto. Also, the if condition could be one line. Also in several other places in this patch. > + > + if ((evtlog_end <= mle_base) && > + (evtlog_base < mle_base)) > + goto pmr_check; /* below */ > + > + sl_txt_reset(SL_ERROR_MLE_BUFFER_OVERLAP); > + > +pmr_check: > + /* > + * The TXT heap is protected by the DPR. If the TPM event log is > + * inside the TXT heap, there is no need for a PMR check. > + */ > + if ((evtlog_base > txt_heap) && > + (evtlog_end < txt_end)) > + return; > + > + sl_check_pmr_coverage(evtlog_base, evtlog_size, true); > +} > +static void sl_process_extend_policy(struct slr_table *slrt) > +{ > + struct slr_entry_policy *policy; > + struct slr_policy_entry *entry; > + u16 i = 0; > + > + policy =(struct slr_entry_policy *) nit: space after '=' ... > +static void sl_process_extend_uefi_config(struct slr_table *slrt) > +{ > + struct slr_entry_uefi_config *uefi_config; > + struct slr_uefi_cfg_entry *uefi_entry; > + u64 i; > + > + uefi_config =(struct slr_entry_uefi_config *) > + slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_UEFI_CONFIG); > + > + /* Optionally here depending on how SL kernel was booted */ > + if (!uefi_config) > + return; > + > + uefi_entry = (struct slr_uefi_cfg_entry *)((u8 *)uefi_config + sizeof(*uefi_config)); > + > + for ( ; i < uefi_config->nr_entries; i++, uefi_entry++) { nit: i seems to be used without first being initialised. > + sl_tpm_extend_evtlog(uefi_entry->pcr, TXT_EVTYPE_SLAUNCH, > + (void *)uefi_entry->cfg, uefi_entry->size, > + uefi_entry->evt_info); > + } > +} > + > +asmlinkage __visible void sl_check_region(void *base, u32 size) > +{ > + sl_check_pmr_coverage(base, size, false); > +} I'm a nit unsure, what to do here, but clang-16 with W=1 says the following. arch/x86/boot/compressed/sl_main.c:533:27: warning: no previous prototype for function 'sl_main' [-Wmissing-prototypes] asmlinkage __visible void sl_main(void *bootparams) ^ arch/x86/boot/compressed/sl_main.c:533:22: note: declare 'static' if the function is not intended to be used outside of this translation unit asmlinkage __visible void sl_main(void *bootparams) ^ static ... > diff --git a/arch/x86/boot/compressed/sl_stub.S b/arch/x86/boot/compressed/sl_stub.S > new file mode 100644 > index 0000000..2d8aa3a > --- /dev/null > +++ b/arch/x86/boot/compressed/sl_stub.S > @@ -0,0 +1,690 @@ > +/* SPDX-License-Identifier: GPL-2.0 */ > + > +/* > + * Secure Launch protected mode entry point. > + * > + * Copyright (c) 2022, Oracle and/or its affiliates. > + */ > + .code32 > + .text > +#include <linux/linkage.h> > +#include <asm/segment.h> > +#include <asm/msr.h> > +#include <asm/processor-flags.h> > +#include <asm/asm-offsets.h> > +#include <asm/bootparam.h> > +#include <asm/page_types.h> > +#include <asm/irq_vectors.h> > +#include <linux/slr_table.h> > +#include <linux/slaunch.h> > + > +/* CPUID: leaf 1, ECX, SMX feature bit */ > +#define X86_FEATURE_BIT_SMX (1 << 6) > + > +/* Can't include apiddef.h in asm */ > +#define XAPIC_ENABLE (1 << 11) > +#define X2APIC_ENABLE (1 << 10) > + > +/* Can't include traps.h in asm */ > +#define X86_TRAP_NMI 2 > + > +/* Can't include mtrr.h in asm */ > +#define MTRRphysBase0 0x200 > + > +#define IDT_VECTOR_LO_BITS 0 > +#define IDT_VECTOR_HI_BITS 6 > + > +/* > + * See the comment in head_64.S for detailed informatoin on what this macro nit: s/informatoin/information/ ... > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h > index 01d19fc..74e3e7df 100644 > --- a/arch/x86/include/uapi/asm/bootparam.h > +++ b/arch/x86/include/uapi/asm/bootparam.h > @@ -26,6 +26,7 @@ > /* loadflags */ > #define LOADED_HIGH (1<<0) > #define KASLR_FLAG (1<<1) > +#define SLAUNCH_FLAG (1<<2) > #define QUIET_FLAG (1<<5) > #define KEEP_SEGMENTS (1<<6) > #define CAN_USE_HEAP (1<<7) nit: please consider using BIT() ...
On 5/5/23 13:47, Simon Horman wrote: > On Thu, May 04, 2023 at 02:50:16PM +0000, Ross Philipson wrote: >> The Secure Launch (SL) stub provides the entry point for Intel TXT (and >> later AMD SKINIT) to vector to during the late launch. The symbol >> sl_stub_entry is that entry point and its offset into the kernel is >> conveyed to the launching code using the MLE (Measured Launch >> Environment) header in the structure named mle_header. The offset of the >> MLE header is set in the kernel_info. The routine sl_stub contains the >> very early late launch setup code responsible for setting up the basic >> environment to allow the normal kernel startup_32 code to proceed. It is >> also responsible for properly waking and handling the APs on Intel >> platforms. The routine sl_main which runs after entering 64b mode is >> responsible for measuring configuration and module information before >> it is used like the boot params, the kernel command line, the TXT heap, >> an external initramfs, etc. >> >> Signed-off-by: Ross Philipson <ross.philipson@oracle.com> > > ... > >> diff --git a/arch/x86/boot/compressed/sl_main.c b/arch/x86/boot/compressed/sl_main.c > > ... > >> +static void *evtlog_base; >> +static u32 evtlog_size; >> +static struct txt_heap_event_log_pointer2_1_element *log20_elem; >> +static u32 tpm_log_ver = SL_TPM12_LOG; >> +struct tcg_efi_specid_event_algs tpm_algs[SL_TPM20_MAX_ALGS] = {0}; > > tpm_algs seems to only be used in this file. > Should it be static? > >> + >> +extern u32 sl_cpu_type; >> +extern u32 sl_mle_start; >> +extern struct boot_params *boot_params; >> + >> +static u64 sl_txt_read(u32 reg) > > Perhaps reg should have an __iomem annotation. > >> +{ >> + return readq((void *)(u64)(TXT_PRIV_CONFIG_REGS_BASE + reg)); >> +} >> + >> +static void sl_txt_write(u32 reg, u64 val) > > Likewise here. > > ... > >> +static void sl_check_pmr_coverage(void *base, u32 size, bool allow_hi) >> +{ >> + struct txt_os_sinit_data *os_sinit_data; >> + void *end = base + size; >> + void *txt_heap; >> + >> + if (!(sl_cpu_type & SL_CPU_INTEL)) >> + return; >> + >> + txt_heap = (void *)sl_txt_read(TXT_CR_HEAP_BASE); >> + os_sinit_data = txt_os_sinit_data_start(txt_heap); >> + >> + if ((end >= (void *)0x100000000ULL) && >> + (base < (void *)0x100000000ULL)) >> + sl_txt_reset(SL_ERROR_REGION_STRADDLE_4GB); >> + >> + /* >> + * Note that the late stub code validates that the hi PMR covers >> + * all memory above 4G. At this point the code can only check that >> + * regions are within the hi PMR but that is sufficient. >> + */ >> + if ((end > (void *)0x100000000ULL) && >> + (base >= (void *)0x100000000ULL)) { >> + if (allow_hi) { >> + if (end >= (void *)(os_sinit_data->vtd_pmr_hi_base + >> + os_sinit_data->vtd_pmr_hi_size)) >> + sl_txt_reset(SL_ERROR_BUFFER_BEYOND_PMR); >> + } else >> + sl_txt_reset(SL_ERROR_REGION_ABOVE_4GB); > > nit: if any arm of a condition has '{}' then all arms should have them. > So: > > } else { > sl_txt_reset(SL_ERROR_REGION_ABOVE_4GB); > } > > Also elsewhere in this patch. > >> + } >> + >> + if (end >= (void *)os_sinit_data->vtd_pmr_lo_size) >> + sl_txt_reset(SL_ERROR_BUFFER_BEYOND_PMR); >> +} >> + >> +/* >> + * Some MSRs are modified by the pre-launch code including the MTRRs. >> + * The early MLE code has to restore these values. This code validates >> + * the values after they are measured. >> + */ >> +static void sl_txt_validate_msrs(struct txt_os_mle_data *os_mle_data) >> +{ >> + struct slr_txt_mtrr_state *saved_bsp_mtrrs; >> + u64 mtrr_caps, mtrr_def_type, mtrr_var; >> + struct slr_entry_intel_info *txt_info; >> + u64 misc_en_msr; >> + u32 vcnt, i; >> + >> + txt_info = (struct slr_entry_intel_info *)os_mle_data->txt_info; >> + saved_bsp_mtrrs = &(txt_info->saved_bsp_mtrrs); > > nit: unnecessary parentheses > > ... > >> +static void sl_validate_event_log_buffer(void) >> +{ >> + struct txt_os_sinit_data *os_sinit_data; >> + void *txt_heap, *txt_end; >> + void *mle_base, *mle_end; >> + void *evtlog_end; >> + >> + if ((u64)evtlog_size > (LLONG_MAX - (u64)evtlog_base)) >> + sl_txt_reset(SL_ERROR_INTEGER_OVERFLOW); >> + evtlog_end = evtlog_base + evtlog_size; >> + >> + txt_heap = (void *)sl_txt_read(TXT_CR_HEAP_BASE); >> + txt_end = txt_heap + sl_txt_read(TXT_CR_HEAP_SIZE); >> + os_sinit_data = txt_os_sinit_data_start(txt_heap); >> + >> + mle_base = (void *)(u64)sl_mle_start; >> + mle_end = mle_base + os_sinit_data->mle_size; >> + >> + /* >> + * This check is to ensure the event log buffer does not overlap with >> + * the MLE image. >> + */ >> + if ((evtlog_base >= mle_end) && >> + (evtlog_end > mle_end)) >> + goto pmr_check; /* above */ > > Ditto. > Also, the if condition could be one line. > Also in several other places in this patch. > >> + >> + if ((evtlog_end <= mle_base) && >> + (evtlog_base < mle_base)) >> + goto pmr_check; /* below */ >> + >> + sl_txt_reset(SL_ERROR_MLE_BUFFER_OVERLAP); >> + >> +pmr_check: >> + /* >> + * The TXT heap is protected by the DPR. If the TPM event log is >> + * inside the TXT heap, there is no need for a PMR check. >> + */ >> + if ((evtlog_base > txt_heap) && >> + (evtlog_end < txt_end)) >> + return; >> + >> + sl_check_pmr_coverage(evtlog_base, evtlog_size, true); >> +} > >> +static void sl_process_extend_policy(struct slr_table *slrt) >> +{ >> + struct slr_entry_policy *policy; >> + struct slr_policy_entry *entry; >> + u16 i = 0; >> + >> + policy =(struct slr_entry_policy *) > > nit: space after '=' > > ... > >> +static void sl_process_extend_uefi_config(struct slr_table *slrt) >> +{ >> + struct slr_entry_uefi_config *uefi_config; >> + struct slr_uefi_cfg_entry *uefi_entry; >> + u64 i; >> + >> + uefi_config =(struct slr_entry_uefi_config *) >> + slr_next_entry_by_tag(slrt, NULL, SLR_ENTRY_UEFI_CONFIG); >> + >> + /* Optionally here depending on how SL kernel was booted */ >> + if (!uefi_config) >> + return; >> + >> + uefi_entry = (struct slr_uefi_cfg_entry *)((u8 *)uefi_config + sizeof(*uefi_config)); >> + >> + for ( ; i < uefi_config->nr_entries; i++, uefi_entry++) { > > nit: i seems to be used without first being initialised. All the ones prior to this we will fix. > >> + sl_tpm_extend_evtlog(uefi_entry->pcr, TXT_EVTYPE_SLAUNCH, >> + (void *)uefi_entry->cfg, uefi_entry->size, >> + uefi_entry->evt_info); >> + } >> +} >> + >> +asmlinkage __visible void sl_check_region(void *base, u32 size) >> +{ >> + sl_check_pmr_coverage(base, size, false); >> +} > > I'm a nit unsure, what to do here, but clang-16 with W=1 says the following. > > arch/x86/boot/compressed/sl_main.c:533:27: warning: no previous prototype for function 'sl_main' [-Wmissing-prototypes] > asmlinkage __visible void sl_main(void *bootparams) > ^ > arch/x86/boot/compressed/sl_main.c:533:22: note: declare 'static' if the function is not intended to be used outside of this translation unit > asmlinkage __visible void sl_main(void *bootparams) > ^ > static Yea we will have to look into why this is. This function is only ever called from asm code so that might have something to do with this. > > ... > >> diff --git a/arch/x86/boot/compressed/sl_stub.S b/arch/x86/boot/compressed/sl_stub.S >> new file mode 100644 >> index 0000000..2d8aa3a >> --- /dev/null >> +++ b/arch/x86/boot/compressed/sl_stub.S >> @@ -0,0 +1,690 @@ >> +/* SPDX-License-Identifier: GPL-2.0 */ >> + >> +/* >> + * Secure Launch protected mode entry point. >> + * >> + * Copyright (c) 2022, Oracle and/or its affiliates. >> + */ >> + .code32 >> + .text >> +#include <linux/linkage.h> >> +#include <asm/segment.h> >> +#include <asm/msr.h> >> +#include <asm/processor-flags.h> >> +#include <asm/asm-offsets.h> >> +#include <asm/bootparam.h> >> +#include <asm/page_types.h> >> +#include <asm/irq_vectors.h> >> +#include <linux/slr_table.h> >> +#include <linux/slaunch.h> >> + >> +/* CPUID: leaf 1, ECX, SMX feature bit */ >> +#define X86_FEATURE_BIT_SMX (1 << 6) >> + >> +/* Can't include apiddef.h in asm */ >> +#define XAPIC_ENABLE (1 << 11) >> +#define X2APIC_ENABLE (1 << 10) >> + >> +/* Can't include traps.h in asm */ >> +#define X86_TRAP_NMI 2 >> + >> +/* Can't include mtrr.h in asm */ >> +#define MTRRphysBase0 0x200 >> + >> +#define IDT_VECTOR_LO_BITS 0 >> +#define IDT_VECTOR_HI_BITS 6 >> + >> +/* >> + * See the comment in head_64.S for detailed informatoin on what this macro > > nit: s/informatoin/information/ > > ... > >> diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h >> index 01d19fc..74e3e7df 100644 >> --- a/arch/x86/include/uapi/asm/bootparam.h >> +++ b/arch/x86/include/uapi/asm/bootparam.h >> @@ -26,6 +26,7 @@ >> /* loadflags */ >> #define LOADED_HIGH (1<<0) >> #define KASLR_FLAG (1<<1) >> +#define SLAUNCH_FLAG (1<<2) >> #define QUIET_FLAG (1<<5) >> #define KEEP_SEGMENTS (1<<6) >> #define CAN_USE_HEAP (1<<7) > > nit: please consider using BIT() I am a little reluctant to change something like this in an existing header. It seems a bit out of scope for the patch set. Thanks Ross > > ...
On Fri, May 05, 2023 at 02:58:28PM -0400, Ross Philipson wrote: > On 5/5/23 13:47, Simon Horman wrote: > > On Thu, May 04, 2023 at 02:50:16PM +0000, Ross Philipson wrote: ... > > > +asmlinkage __visible void sl_check_region(void *base, u32 size) > > > +{ > > > + sl_check_pmr_coverage(base, size, false); > > > +} > > > > I'm a nit unsure, what to do here, but clang-16 with W=1 says the following. > > > > arch/x86/boot/compressed/sl_main.c:533:27: warning: no previous prototype for function 'sl_main' [-Wmissing-prototypes] > > asmlinkage __visible void sl_main(void *bootparams) > > ^ > > arch/x86/boot/compressed/sl_main.c:533:22: note: declare 'static' if the function is not intended to be used outside of this translation unit > > asmlinkage __visible void sl_main(void *bootparams) > > ^ > > static > > Yea we will have to look into why this is. This function is only ever called > from asm code so that might have something to do with this. Thanks. ... > > > diff --git a/arch/x86/include/uapi/asm/bootparam.h b/arch/x86/include/uapi/asm/bootparam.h > > > index 01d19fc..74e3e7df 100644 > > > --- a/arch/x86/include/uapi/asm/bootparam.h > > > +++ b/arch/x86/include/uapi/asm/bootparam.h > > > @@ -26,6 +26,7 @@ > > > /* loadflags */ > > > #define LOADED_HIGH (1<<0) > > > #define KASLR_FLAG (1<<1) > > > +#define SLAUNCH_FLAG (1<<2) > > > #define QUIET_FLAG (1<<5) > > > #define KEEP_SEGMENTS (1<<6) > > > #define CAN_USE_HEAP (1<<7) > > > > nit: please consider using BIT() > > I am a little reluctant to change something like this in an existing header. > It seems a bit out of scope for the patch set. Yes, sorry for the noise on this one. I agree that what you have is the best approach here.
On 5/5/23 22:45, Ross Philipson wrote: > Sorry about that. In the future I will include a base-commit field. It is based off of torvolds/master as of 5/1/2023. The branch where the patches came from is now pushed to the TrenchBoot repository here: > > https://github.com/TrenchBoot/linux/tree/linux-sl-master-5-1-23-v6 > Pulled, thanks!
On 5/5/23 12:34, Simon Horman wrote: > On Thu, May 04, 2023 at 02:50:15PM +0000, Ross Philipson wrote: >> From: "Daniel P. Smith" <dpsmith@apertussolutions.com> >> >> The SHA algorithms are necessary to measure configuration information into >> the TPM as early as possible before using the values. This implementation >> uses the established approach of #including the SHA libraries directly in >> the code since the compressed kernel is not uncompressed at this point. >> >> The SHA code here has its origins in the code from the main kernel: >> >> commit c4d5b9ffa31f ("crypto: sha1 - implement base layer for SHA-1") >> >> That code could not be pulled directly into the setup portion of the >> compressed kernel because of other dependencies it pulls in. The result >> is this is a modified copy of that code that still leverages the core >> SHA algorithms. >> >> Signed-off-by: Daniel P. Smith <dpsmith@apertussolutions.com> >> Signed-off-by: Ross Philipson <ross.philipson@oracle.com> >> --- >> arch/x86/boot/compressed/Makefile | 2 + >> arch/x86/boot/compressed/early_sha1.c | 97 +++++++++++++++++++++++++++++++++ >> arch/x86/boot/compressed/early_sha1.h | 17 ++++++ >> arch/x86/boot/compressed/early_sha256.c | 7 +++ >> lib/crypto/sha1.c | 4 ++ >> lib/crypto/sha256.c | 8 +++ >> 6 files changed, 135 insertions(+) >> create mode 100644 arch/x86/boot/compressed/early_sha1.c >> create mode 100644 arch/x86/boot/compressed/early_sha1.h >> create mode 100644 arch/x86/boot/compressed/early_sha256.c >> >> diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile >> index 6b6cfe6..1d327d4 100644 >> --- a/arch/x86/boot/compressed/Makefile >> +++ b/arch/x86/boot/compressed/Makefile >> @@ -112,6 +112,8 @@ vmlinux-objs-$(CONFIG_EFI) += $(obj)/efi.o >> vmlinux-objs-$(CONFIG_EFI_MIXED) += $(obj)/efi_mixed.o >> vmlinux-objs-$(CONFIG_EFI_STUB) += $(objtree)/drivers/firmware/efi/libstub/lib.a >> >> +vmlinux-objs-$(CONFIG_SECURE_LAUNCH) += $(obj)/early_sha1.o $(obj)/early_sha256.o >> + >> $(obj)/vmlinux: $(vmlinux-objs-y) FORCE >> $(call if_changed,ld) >> >> diff --git a/arch/x86/boot/compressed/early_sha1.c b/arch/x86/boot/compressed/early_sha1.c >> new file mode 100644 >> index 0000000..524ec23 >> --- /dev/null >> +++ b/arch/x86/boot/compressed/early_sha1.c >> @@ -0,0 +1,97 @@ >> +// SPDX-License-Identifier: GPL-2.0 >> +/* >> + * Copyright (c) 2022 Apertus Solutions, LLC. >> + */ >> + >> +#include <linux/init.h> >> +#include <linux/linkage.h> >> +#include <linux/string.h> >> +#include <asm/boot.h> >> +#include <asm/unaligned.h> >> + >> +#include "early_sha1.h" >> + >> +#define SHA1_DISABLE_EXPORT > > Hi Ross, > > I could be mistaken, but it seems to me that *_DISABLE_EXPORT > is a mechanism of this patch to disable exporting symbols > (use of EXPORT_SYMBOL), at compile time. > > If so, this doesn't strike me as something that should be part of upstream > kernel code. Could you consider dropping this part of the patch? Greetings Simon, This was patterned after the move of sha256 to /lib. Upon re-inspection, it appears this has since been updated to use the __DISABLE_EXPORTS CFLAG mechanism of EXPORT_SYMBOL as a conditionally included rule in the Makefile where the desire to disable exporting is wanted. We will update this patch to follow the same pattern. V/r, Daniel P. Smith
The larger focus of the TrenchBoot project (https://github.com/TrenchBoot) is to enhance the boot security and integrity in a unified manner. The first area of focus has been on the Trusted Computing Group's Dynamic Launch for establishing a hardware Root of Trust for Measurement, also know as DRTM (Dynamic Root of Trust for Measurement). The project has been and continues to work on providing a unified means to Dynamic Launch that is a cross-platform (Intel and AMD) and cross-architecture (x86 and Arm), with our recent involvment in the upcoming Arm DRTM specification. The order of introducing DRTM to the Linux kernel follows the maturity of DRTM in the architectures. Intel's Trusted eXecution Technology (TXT) is present today and only requires a preamble loader, e.g. a boot loader, and an OS kernel that is TXT-aware. AMD DRTM implementation has been present since the introduction of AMD-V but requires an additional component that is AMD specific and referred to in the specification as the Secure Loader, which the TrenchBoot project has an active prototype in development. Finally Arm's implementation is in specification development stage and the project is looking to support it when it becomes available. This patchset provides detailed documentation of DRTM, the approach used for adding the capbility, and relevant API/ABI documentation. In addition to the documentation the patch set introduces Intel TXT support as the first platform for Linux Secure Launch. A quick note on terminology. The larger open source project itself is called TrenchBoot, which is hosted on Github (links below). The kernel feature enabling the use of Dynamic Launch technology is referred to as "Secure Launch" within the kernel code. As such the prefixes sl_/SL_ or slaunch/SLAUNCH will be seen in the code. The stub code discussed above is referred to as the SL stub. The Secure Launch feature starts with patch #2. Patch #1 was authored by Arvind Sankar. There is no further status on this patch at this point but Secure Launch depends on it so it is included with the set. Links: The TrenchBoot project including documentation: https://trenchboot.org The TrenchBoot project on Github: https://github.com/trenchboot Intel TXT is documented in its own specification and in the SDM Instruction Set volume: https://www.intel.com/content/dam/www/public/us/en/documents/guides/intel-txt-software-development-guide.pdf https://software.intel.com/en-us/articles/intel-sdm AMD SKINIT is documented in the System Programming manual: https://www.amd.com/system/files/TechDocs/24593.pdf GRUB2 pre-launch support branch (WIP): https://github.com/TrenchBoot/grub/tree/grub-sl-fc-38-dlstub Thanks Ross Philipson and Daniel P. Smith Changes in v2: - Modified 32b entry code to prevent causing relocations in the compressed kernel. - Dropped patches for compressed kernel TPM PCR extender. - Modified event log code to insert log delimiter events and not rely on TPM access. - Stop extending PCRs in the early Secure Launch stub code. - Removed Kconfig options for hash algorithms and use the algorithms the ACM used. - Match Secure Launch measurement algorithm use to those reported in the TPM 2.0 event log. - Read the TPM events out of the TPM and extend them into the PCRs using the mainline TPM driver. This is done in the late initcall module. - Allow use of alternate PCR 19 and 20 for post ACM measurements. - Add Kconfig constraints needed by Secure Launch (disable KASLR and add x2apic dependency). - Fix testing of SL_FLAGS when determining if Secure Launch is active and the architecture is TXT. - Use SYM_DATA_START_LOCAL macros in early entry point code. - Security audit changes: - Validate buffers passed to MLE do not overlap the MLE and are properly laid out. - Validate buffers and memory regions used by the MLE are protected by IOMMU PMRs. - Force IOMMU to not use passthrough mode during a Secure Launch. - Prevent KASLR use during a Secure Launch. Changes in v3: - Introduce x86 documentation patch to provide background, overview and configuration/ABI information for the Secure Launch kernel feature. - Remove the IOMMU patch with special cases for disabling IOMMU passthrough. Configuring the IOMMU is now a documentation matter in the previously mentioned new patch. - Remove special case KASLR disabling code. Configuring KASLR is now a documentation matter in the previously mentioned new patch. - Fix incorrect panic on TXT public register read. - Properly handle and measure setup_indirect bootparams in the early launch code. - Use correct compressed kernel image base address when testing buffers in the early launch stub code. This bug was introduced by the changes to avoid relocation in the compressed kernel. - Use CPUID feature bits instead of CPUID vendor strings to determine if SMX mode is supported and the system is Intel. - Remove early NMI re-enable on the BSP. This can be safely done later on the BSP after an IDT is setup. Changes in v4: - Expand the cover letter to provide more context to the order that DRTM support will be added. - Removed debug tracing in TPM request locality funciton and fixed local variable declarations. - Fixed missing break in default case in slmodule.c. - Reworded commit messages in patches 1 and 2 per suggestions. Changes in v5: - Comprehensive documentation rewrite. - Use boot param loadflags to communicate Secure Launch status to kernel proper. - Fix incorrect check of X86_FEATURE_BIT_SMX bit. - Rename the alternate details and authorities PCR support. - Refactor the securityfs directory and file setup in slmodule.c. - Misc. cleanup from internal code reviews. - Use reverse fir tree format for variables. Changes in v6: - Support for the new Secure Launch Resourse Table that standardizes the information passed and forms the ABI between the pre and post launch code. - Support for booting Linux through the EFI stub entry point and then being able to do a Secure Launch once EFI stub is done and EBS is called. - Updates to the documentation to reflect the previous two items listed. Arvind Sankar (1): x86/boot: Place kernel_info at a fixed offset Daniel P. Smith (2): x86: Add early SHA support for Secure Launch early measurements x86: Secure Launch late initcall platform module Ross Philipson (11): Documentation/x86: Secure Launch kernel documentation x86: Secure Launch Kconfig x86: Secure Launch Resource Table header file x86: Secure Launch main header file x86: Secure Launch kernel early boot stub x86: Secure Launch kernel late boot stub x86: Secure Launch SMP bringup support kexec: Secure Launch kexec SEXIT support reboot: Secure Launch SEXIT support on reboot paths tpm: Allow locality 2 to be set when initializing the TPM for Secure Launch x86: EFI stub DRTM launch support for Secure Launch Documentation/arch/x86/boot.rst | 21 + Documentation/security/index.rst | 1 + Documentation/security/launch-integrity/index.rst | 10 + .../security/launch-integrity/principles.rst | 313 ++++++++++ .../launch-integrity/secure_launch_details.rst | 564 +++++++++++++++++ .../launch-integrity/secure_launch_overview.rst | 220 +++++++ arch/x86/Kconfig | 12 + arch/x86/boot/compressed/Makefile | 3 + arch/x86/boot/compressed/early_sha1.c | 97 +++ arch/x86/boot/compressed/early_sha1.h | 17 + arch/x86/boot/compressed/early_sha256.c | 7 + arch/x86/boot/compressed/head_64.S | 37 ++ arch/x86/boot/compressed/kernel_info.S | 53 +- arch/x86/boot/compressed/kernel_info.h | 12 + arch/x86/boot/compressed/sl_main.c | 599 ++++++++++++++++++ arch/x86/boot/compressed/sl_stub.S | 690 +++++++++++++++++++++ arch/x86/boot/compressed/vmlinux.lds.S | 6 + arch/x86/include/asm/realmode.h | 3 + arch/x86/include/uapi/asm/bootparam.h | 1 + arch/x86/kernel/Makefile | 2 + arch/x86/kernel/asm-offsets.c | 20 + arch/x86/kernel/reboot.c | 10 + arch/x86/kernel/setup.c | 3 + arch/x86/kernel/slaunch.c | 566 +++++++++++++++++ arch/x86/kernel/slmodule.c | 520 ++++++++++++++++ arch/x86/kernel/smpboot.c | 86 +++ arch/x86/realmode/rm/header.S | 3 + arch/x86/realmode/rm/trampoline_64.S | 37 ++ drivers/char/tpm/tpm-chip.c | 9 +- drivers/firmware/efi/libstub/x86-stub.c | 55 ++ drivers/iommu/intel/dmar.c | 4 + include/linux/slaunch.h | 513 +++++++++++++++ include/linux/slr_table.h | 270 ++++++++ kernel/kexec_core.c | 4 + lib/crypto/sha1.c | 4 + lib/crypto/sha256.c | 8 + 36 files changed, 4775 insertions(+), 5 deletions(-) create mode 100644 Documentation/security/launch-integrity/index.rst create mode 100644 Documentation/security/launch-integrity/principles.rst create mode 100644 Documentation/security/launch-integrity/secure_launch_details.rst create mode 100644 Documentation/security/launch-integrity/secure_launch_overview.rst create mode 100644 arch/x86/boot/compressed/early_sha1.c create mode 100644 arch/x86/boot/compressed/early_sha1.h create mode 100644 arch/x86/boot/compressed/early_sha256.c create mode 100644 arch/x86/boot/compressed/kernel_info.h create mode 100644 arch/x86/boot/compressed/sl_main.c create mode 100644 arch/x86/boot/compressed/sl_stub.S create mode 100644 arch/x86/kernel/slaunch.c create mode 100644 arch/x86/kernel/slmodule.c create mode 100644 include/linux/slaunch.h create mode 100644 include/linux/slr_table.h