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
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAMj1kXHu_P9Wn5sBejcSCKedAnxKzcjJGuo4jgbRc-Pr9FVgEQ@mail.gmail.com>
Date: Fri, 31 May 2024 18:13:24 +0200
From: Ard Biesheuvel <ardb@...nel.org>
To: Ross Philipson <ross.philipson@...cle.com>
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, mjg59@...f.ucam.org, 
	James.Bottomley@...senpartnership.com, peterhuewe@....de, jarkko@...nel.org, 
	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 v9 08/19] x86: Secure Launch kernel early boot stub

On Fri, 31 May 2024 at 16:04, Ard Biesheuvel <ardb@...nel.org> wrote:
>
> On Fri, 31 May 2024 at 15:33, Ard Biesheuvel <ardb@...nel.org> wrote:
> >
> > On Fri, 31 May 2024 at 13:00, Ard Biesheuvel <ardb@...nel.org> wrote:
> > >
> > > Hello Ross,
> > >
> > > On Fri, 31 May 2024 at 03:32, Ross Philipson <ross.philipson@...cle.com> 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@...cle.com>
> > > > ---
> > > >  Documentation/arch/x86/boot.rst        |  21 +
> > > >  arch/x86/boot/compressed/Makefile      |   3 +-
> > > >  arch/x86/boot/compressed/head_64.S     |  30 +
> > > >  arch/x86/boot/compressed/kernel_info.S |  34 ++
> > > >  arch/x86/boot/compressed/sl_main.c     | 577 ++++++++++++++++++++
> > > >  arch/x86/boot/compressed/sl_stub.S     | 725 +++++++++++++++++++++++++
> > > >  arch/x86/include/asm/msr-index.h       |   5 +
> > > >  arch/x86/include/uapi/asm/bootparam.h  |   1 +
> > > >  arch/x86/kernel/asm-offsets.c          |  20 +
> > > >  9 files changed, 1415 insertions(+), 1 deletion(-)
> > > >  create mode 100644 arch/x86/boot/compressed/sl_main.c
> > > >  create mode 100644 arch/x86/boot/compressed/sl_stub.S
> > > >
> > > > diff --git a/Documentation/arch/x86/boot.rst b/Documentation/arch/x86/boot.rst
> > > > index 4fd492cb4970..295cdf9bcbdb 100644
> > > > --- a/Documentation/arch/x86/boot.rst
> > > > +++ b/Documentation/arch/x86/boot.rst
> > > > @@ -482,6 +482,14 @@ Protocol:  2.00+
> > > >             - If 1, KASLR enabled.
> > > >             - If 0, KASLR disabled.
> > > >
> > > > +  Bit 2 (kernel internal): SLAUNCH_FLAG
> > > > +
> > > > +       - Used internally by the setup kernel to communicate
> > > > +         Secure Launch status to kernel proper.
> > > > +
> > > > +           - If 1, Secure Launch enabled.
> > > > +           - If 0, Secure Launch disabled.
> > > > +
> > > >    Bit 5 (write): QUIET_FLAG
> > > >
> > > >         - If 0, print early messages.
> > > > @@ -1028,6 +1036,19 @@ Offset/size:     0x000c/4
> > > >
> > > >    This field contains maximal allowed type for setup_data and setup_indirect structs.
> > > >
> > > > +============   =================
> > > > +Field name:    mle_header_offset
> > > > +Offset/size:   0x0010/4
> > > > +============   =================
> > > > +
> > > > +  This field contains the offset to the Secure Launch Measured Launch Environment
> > > > +  (MLE) header. This offset is used to locate information needed during a secure
> > > > +  late launch using Intel TXT. If the offset is zero, the kernel does not have
> > > > +  Secure Launch capabilities. The MLE entry point is called from TXT on the BSP
> > > > +  following a success measured launch. The specific state of the processors is
> > > > +  outlined in the TXT Software Development Guide, the latest can be found here:
> > > > +  https://www.intel.com/content/dam/www/public/us/en/documents/guides/intel-txt-software-development-guide.pdf
> > > > +
> > > >
> > >
> > > Could we just repaint this field as the offset relative to the start
> > > of kernel_info rather than relative to the start of the image? That
> > > way, there is no need for patch #1, and given that the consumer of
> > > this field accesses it via kernel_info, I wouldn't expect any issues
> > > in applying this offset to obtain the actual address.
> > >
> > >
> > > >  The Image Checksum
> > > >  ==================
> > > > diff --git a/arch/x86/boot/compressed/Makefile b/arch/x86/boot/compressed/Makefile
> > > > index 9189a0e28686..9076a248d4b4 100644
> > > > --- a/arch/x86/boot/compressed/Makefile
> > > > +++ b/arch/x86/boot/compressed/Makefile
> > > > @@ -118,7 +118,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
> > > > +vmlinux-objs-$(CONFIG_SECURE_LAUNCH) += $(obj)/early_sha1.o $(obj)/early_sha256.o \
> > > > +       $(obj)/sl_main.o $(obj)/sl_stub.o
> > > >
> > > >  $(obj)/vmlinux: $(vmlinux-objs-y) FORCE
> > > >         $(call if_changed,ld)
> > > > diff --git a/arch/x86/boot/compressed/head_64.S b/arch/x86/boot/compressed/head_64.S
> > > > index 1dcb794c5479..803c9e2e6d85 100644
> > > > --- a/arch/x86/boot/compressed/head_64.S
> > > > +++ b/arch/x86/boot/compressed/head_64.S
> > > > @@ -420,6 +420,13 @@ SYM_CODE_START(startup_64)
> > > >         pushq   $0
> > > >         popfq
> > > >
> > > > +#ifdef CONFIG_SECURE_LAUNCH
> > > > +       /* Ensure the relocation region is coverd by a PMR */
> > >
> > > covered
> > >
> > > > +       movq    %rbx, %rdi
> > > > +       movl    $(_bss - startup_32), %esi
> > > > +       callq   sl_check_region
> > > > +#endif
> > > > +
> > > >  /*
> > > >   * Copy the compressed kernel to the end of our buffer
> > > >   * where decompression in place becomes safe.
> > > > @@ -462,6 +469,29 @@ SYM_FUNC_START_LOCAL_NOALIGN(.Lrelocated)
> > > >         shrq    $3, %rcx
> > > >         rep     stosq
> > > >
> > > > +#ifdef CONFIG_SECURE_LAUNCH
> > > > +       /*
> > > > +        * Have to do the final early sl stub work in 64b area.
> > > > +        *
> > > > +        * *********** NOTE ***********
> > > > +        *
> > > > +        * Several boot params get used before we get a chance to measure
> > > > +        * them in this call. This is a known issue and we currently don't
> > > > +        * have a solution. The scratch field doesn't matter. There is no
> > > > +        * obvious way to do anything about the use of kernel_alignment or
> > > > +        * init_size though these seem low risk with all the PMR and overlap
> > > > +        * checks in place.
> > > > +        */
> > > > +       movq    %r15, %rdi
> > > > +       callq   sl_main
> > > > +
> > > > +       /* Ensure the decompression location is covered by a PMR */
> > > > +       movq    %rbp, %rdi
> > > > +       movq    output_len(%rip), %rsi
> > > > +       callq   sl_check_region
> > > > +#endif
> > > > +
> > > > +       pushq   %rsi
> > >
> > > This looks like a rebase error.
> > >
> > > >         call    load_stage2_idt
> > > >
> > > >         /* Pass boot_params to initialize_identity_maps() */
> > > > diff --git a/arch/x86/boot/compressed/kernel_info.S b/arch/x86/boot/compressed/kernel_info.S
> > > > index c18f07181dd5..e199b87764e9 100644
> > > > --- a/arch/x86/boot/compressed/kernel_info.S
> > > > +++ b/arch/x86/boot/compressed/kernel_info.S
> > > > @@ -28,6 +28,40 @@ SYM_DATA_START(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   rva(mle_header)
> > >
> > > ... so this could just be mle_header - kernel_info, and the consumer
> > > can do the math instead.
> > >
> > > > +#else
> > > > +       .long   0
> > > > +#endif
> > > > +
> > > >  kernel_info_var_len_data:
> > > >         /* Empty for time being... */
> > > >  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   rva(sl_stub_entry) /* Linear entry point of MLE (virt. address) */
> > >
> > > and these should perhaps be relative to mle_header?
> > >
> > > > +       .long   0x00000000  /* First valid page of MLE */
> > > > +       .long   0x00000000  /* Offset within binary of first byte of MLE */
> > > > +       .long   rva(_edata) /* Offset within binary of last byte + 1 of MLE */
> > >
> > > and here
> > >
> >
> > Ugh never mind - these are specified externally.
>
> OK, so instead of patch #1, please use the linker script to generate
> these constants.
>
> I.e., add this to arch/x86/boot/compressed/vmlinux.lds.S
>
> #ifdef CONFIG_SECURE_LAUNCH
> PROVIDE(mle_header_offset       = mle_header - startup_32);
> PROVIDE(sl_stub_entry_offset    = sl_stub_entry - startup_32);
> PROVIDE(_edata_offset           = _edata - startup_32);
> #endif
>
> and use the symbols on the left hand side in the code.

Bah this works with Clang but not with GCC/ld.bfd

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ