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: <e68edaab-e7cf-4a13-acb1-87ddbb73765a@oracle.com>
Date: Thu, 24 Apr 2025 11:56:11 -0700
From: ross.philipson@...cle.com
To: "Huang, Kai" <kai.huang@...el.com>,
        "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>
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 4/24/25 5:29 AM, Huang, Kai wrote:
> 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>?

Yes this looks to be a good idea. I think it has been something we have 
thought of before. We will look into it.


> 
> 
> [...]
> 
>> +/*
>> + * 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?

If we do what you suggest, I think these ambiguities will go away.

Thank you,
Ross