linux-kernel - Re: [PATCH v14 04/19] x86: Secure Launch main header file

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: <e2d00d7cc4cfc33a3b328fe8386f6d164bc46184.camel@intel.com>
Date: Thu, 24 Apr 2025 12:29:18 +0000
From: "Huang, Kai" <kai.huang@...el.com>
To: "kexec@...ts.infradead.org" <kexec@...ts.infradead.org>, "x86@...nel.org"
	<x86@...nel.org>, "linux-integrity@...r.kernel.org"
	<linux-integrity@...r.kernel.org>, "iommu@...ts.linux.dev"
	<iommu@...ts.linux.dev>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>, "linux-efi@...r.kernel.org"
	<linux-efi@...r.kernel.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>, "linux-crypto@...r.kernel.org"
	<linux-crypto@...r.kernel.org>, "ross.philipson@...cle.com"
	<ross.philipson@...cle.com>
CC: "corbet@....net" <corbet@....net>, "ardb@...nel.org" <ardb@...nel.org>,
	"andrew.cooper3@...rix.com" <andrew.cooper3@...rix.com>, "peterhuewe@....de"
	<peterhuewe@....de>, "James.Bottomley@...senpartnership.com"
	<James.Bottomley@...senpartnership.com>, "trenchboot-devel@...glegroups.com"
	<trenchboot-devel@...glegroups.com>, "dave.hansen@...ux.intel.com"
	<dave.hansen@...ux.intel.com>, "davem@...emloft.net" <davem@...emloft.net>,
	"baolu.lu@...ux.intel.com" <baolu.lu@...ux.intel.com>,
	"herbert@...dor.apana.org.au" <herbert@...dor.apana.org.au>,
	"mingo@...hat.com" <mingo@...hat.com>, "Ghatraju, Kanth"
	<kanth.ghatraju@...cle.com>, "tglx@...utronix.de" <tglx@...utronix.de>,
	"luto@...capital.net" <luto@...capital.net>, "dwmw2@...radead.org"
	<dwmw2@...radead.org>, "dpsmith@...rtussolutions.com"
	<dpsmith@...rtussolutions.com>, "jarkko@...nel.org" <jarkko@...nel.org>,
	"ebiederm@...ssion.com" <ebiederm@...ssion.com>, "hpa@...or.com"
	<hpa@...or.com>, "bp@...en8.de" <bp@...en8.de>, "mjg59@...f.ucam.org"
	<mjg59@...f.ucam.org>, "jgg@...pe.ca" <jgg@...pe.ca>, "nivedita@...m.mit.edu"
	<nivedita@...m.mit.edu>
Subject: Re: [PATCH v14 04/19] x86: Secure Launch main header file

On Mon, 2025-04-21 at 09:26 -0700, Ross Philipson wrote:
> Introduce the main Secure Launch header file used in the early SL stub
> and the early setup code.
> 
> This header file contains the following categories:
>  - Secure Launch implementation specific structures and definitions.
>  - Intel TXT architecture specific DRTM structures, definitions and functions
> used by Secure Launch.
>  - DRTM TPM event logging definitions and helper functions.

Looking at the actual code in this patch, seems >90% code in the
<linux/slaunch.h> is Intel specific, e.g., TXT specific macro/structure
definitions.  It doesn't seem to be the right way to organize the code.

E.g., following the current pattern, when we need to add support for another TXT
similar vendor-specific technology, we will need to add yet-another set of
macro/structure definitions for that technology to <linux/slaunch.h> as well.

That would be a giant mess IMHO.

Could we make <linux/slaunch.h> only contain the common things, and move Intel
specific things to some x86 header(s), e.g., <asm/intel-txt.h> or <asm/txt.h>?


[...]

> +/*
> + * External functions available in mainline kernel.
> + */
> +void slaunch_setup_txt(void);
> +void slaunch_fixup_jump_vector(void);
> +u32 slaunch_get_flags(void);
> +struct sl_ap_wake_info *slaunch_get_ap_wake_info(void);
> +struct acpi_table_header *slaunch_get_dmar_table(struct acpi_table_header *dmar);
> +void __noreturn slaunch_txt_reset(void __iomem *txt,
> +					 const char *msg, u64 error);
> +void slaunch_finalize(int do_sexit);
> +
> +static inline bool slaunch_is_txt_launch(void)
> +{
> +	u32 mask = SL_FLAG_ACTIVE | SL_FLAG_ARCH_TXT;
> +
> +	return (slaunch_get_flags() & mask) == mask;
> +}
> +
> +#else
> +
> +static inline void slaunch_setup_txt(void)
> +{
> +}
> +
> +static inline void slaunch_fixup_jump_vector(void)
> +{
> +}
> +
> +static inline u32 slaunch_get_flags(void)
> +{
> +	return 0;
> +}
> +
> +static inline struct acpi_table_header *slaunch_get_dmar_table(struct acpi_table_header *dmar)
> +{
> +	return dmar;
> +}
> +
> +static inline void slaunch_finalize(int do_sexit)
> +{
> +}
> +
> +static inline bool slaunch_is_txt_launch(void)
> +{
> +	return false;
> +}
> 

.. btw it's not clear which part of the code is common code.  

Looking at the abvoe code, it seems those functions are common functions called
from common code.  E.g., slaunch_finalize() is called from kernel/kexec_core.c,
which means it is a concept in the kernel common code and must be available for
all ARCHs (I haven't read how other functions are called, though).

But the name slaunch_setup_txt(), slaunch_get_dmar_table() and
slaunch_is_txt_launch() are quite Intel specific.  So it seems to me this patch
_tries_ to support Secure Launch at the arch agnostic level but it doesn't do a
good job at the abstraction?