| 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: <6e1e771c-ecd4-4ab6-ba9f-900f34c5d89d@oracle.com>
Date: Wed, 4 Feb 2026 09:44:34 -0800
From: ross.philipson@...cle.com
To: "Daniel P. Smith" <dpsmith@...rtussolutions.com>,
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.dev, 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 v15 02/28] tpm: Move TPM1 specific definitions and
functions to new headers
On 2/1/26 8:23 AM, Daniel P. Smith wrote:
> On 1/19/26 18:57, Jarkko Sakkinen wrote:
>> On Mon, Dec 15, 2025 at 03:32:50PM -0800, Ross Philipson wrote:
>>> This gathers all the TPM1 definitions and structures into two separate
>>> header files (public tpm1.h and private tpm1_structs.h). The definitions
>>> moved to these files correspond to the TCG specification for TPM 1 family:
>>>
>>> TPM 1.2 Main Specification
>>> - https://urldefense.com/v3/__https://trustedcomputinggroup.org/resource/tpm-main-specification/__;!!ACWV5N9M2RV99hQ!OivTxO-R0nvBJYXIV-T0n0fq0wY64MTgdaecPzAauxrFkRxTcQ6CuBEcXmZZYY1KfZlqmfLTZBbPaEr7Y8k7hJk9_mQ$
>>> Note that the structures were pulled into tpm1_structs.h to allow their
>>> external reuse.
>>>
>>> Signed-off-by: Daniel P. Smith <dpsmith@...rtussolutions.com>
>>> Signed-off-by: Ross Philipson <ross.philipson@...cle.com>
>>> ---
>>> drivers/char/tpm/tpm.h | 98 +--------------------------------
>>> drivers/char/tpm/tpm1-cmd.c | 5 --
>>> drivers/char/tpm/tpm1_structs.h | 97 ++++++++++++++++++++++++++++++++
>>
>> I think you are overcomplicating the patch set and doing more
>> than you really need to do.
>>
>> I.e. structs could go also just as well to tpm_command.h. We
>> will deal with if/when that file ever grows too large. It's
>> absolutely not a priority for this patch set.
>
> Ack.
>
>>> include/linux/tpm1.h | 34 +++++++++++-
>>> 4 files changed, 132 insertions(+), 102 deletions(-)
>>> create mode 100644 drivers/char/tpm/tpm1_structs.h
>>>
>>> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
>>> index ca391b2a211c..1f9f8540eede 100644
>>> --- a/drivers/char/tpm/tpm.h
>>> +++ b/drivers/char/tpm/tpm.h
>>> @@ -50,105 +50,9 @@ enum tpm_addr {
>>> TPM_ADDR = 0x4E,
>>> };
>>> -#define TPM_WARN_RETRY 0x800
>>> -#define TPM_WARN_DOING_SELFTEST 0x802
>>> -#define TPM_ERR_DEACTIVATED 0x6
>>> -#define TPM_ERR_DISABLED 0x7
>>> -#define TPM_ERR_FAILEDSELFTEST 0x1C
>>> -#define TPM_ERR_INVALID_POSTINIT 38
>>> -
>>> -#define TPM_TAG_RQU_COMMAND 193
>>> -
>>> /* TPM2 specific constants. */
>>> #define TPM2_SPACE_BUFFER_SIZE 16384 /* 16 kB */
>>> -struct stclear_flags_t {
>>> - __be16 tag;
>>> - u8 deactivated;
>>> - u8 disableForceClear;
>>> - u8 physicalPresence;
>>> - u8 physicalPresenceLock;
>>> - u8 bGlobalLock;
>>> -} __packed;
>>> -
>>> -struct tpm1_version {
>>> - u8 major;
>>> - u8 minor;
>>> - u8 rev_major;
>>> - u8 rev_minor;
>>> -} __packed;
>>> -
>>> -struct tpm1_version2 {
>>> - __be16 tag;
>>> - struct tpm1_version version;
>>> -} __packed;
>>> -
>>> -struct timeout_t {
>>> - __be32 a;
>>> - __be32 b;
>>> - __be32 c;
>>> - __be32 d;
>>> -} __packed;
>>> -
>>> -struct duration_t {
>>> - __be32 tpm_short;
>>> - __be32 tpm_medium;
>>> - __be32 tpm_long;
>>> -} __packed;
>>> -
>>> -struct permanent_flags_t {
>>> - __be16 tag;
>>> - u8 disable;
>>> - u8 ownership;
>>> - u8 deactivated;
>>> - u8 readPubek;
>>> - u8 disableOwnerClear;
>>> - u8 allowMaintenance;
>>> - u8 physicalPresenceLifetimeLock;
>>> - u8 physicalPresenceHWEnable;
>>> - u8 physicalPresenceCMDEnable;
>>> - u8 CEKPUsed;
>>> - u8 TPMpost;
>>> - u8 TPMpostLock;
>>> - u8 FIPS;
>>> - u8 operator;
>>> - u8 enableRevokeEK;
>>> - u8 nvLocked;
>>> - u8 readSRKPub;
>>> - u8 tpmEstablished;
>>> - u8 maintenanceDone;
>>> - u8 disableFullDALogicInfo;
>>> -} __packed;
>>> -
>>> -typedef union {
>>> - struct permanent_flags_t perm_flags;
>>> - struct stclear_flags_t stclear_flags;
>>> - __u8 owned;
>>> - __be32 num_pcrs;
>>> - struct tpm1_version version1;
>>> - struct tpm1_version2 version2;
>>> - __be32 manufacturer_id;
>>> - struct timeout_t timeout;
>>> - struct duration_t duration;
>>> -} cap_t;
>>> -
>>> -enum tpm_capabilities {
>>> - TPM_CAP_FLAG = 4,
>>> - TPM_CAP_PROP = 5,
>>> - TPM_CAP_VERSION_1_1 = 0x06,
>>> - TPM_CAP_VERSION_1_2 = 0x1A,
>>> -};
>>> -
>>> -enum tpm_sub_capabilities {
>>> - TPM_CAP_PROP_PCR = 0x101,
>>> - TPM_CAP_PROP_MANUFACTURER = 0x103,
>>> - TPM_CAP_FLAG_PERM = 0x108,
>>> - TPM_CAP_FLAG_VOL = 0x109,
>>> - TPM_CAP_PROP_OWNER = 0x111,
>>> - TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
>>> - TPM_CAP_PROP_TIS_DURATION = 0x120,
>>> -};
>>> -
>>> enum tpm2_pt_props {
>>> TPM2_PT_NONE = 0x00000000,
>>> TPM2_PT_GROUP = 0x00000100,
>>> @@ -229,6 +133,8 @@ enum tpm2_pt_props {
>>> * compiler warnings about stack frame size. */
>>> #define TPM_MAX_RNG_DATA 128
>>> +#include "tpm1_structs.h"
>>> +
>>> extern const struct class tpm_class;
>>> extern const struct class tpmrm_class;
>>> extern dev_t tpm_devt;
>>> diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c
>>> index f29827b454d2..02f20a0aa37d 100644
>>> --- a/drivers/char/tpm/tpm1-cmd.c
>>> +++ b/drivers/char/tpm/tpm1-cmd.c
>>> @@ -505,11 +505,6 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap,
>>> }
>>> EXPORT_SYMBOL_GPL(tpm1_getcap);
>>> -struct tpm1_get_random_out {
>>> - __be32 rng_data_len;
>>> - u8 rng_data[TPM_MAX_RNG_DATA];
>>> -} __packed;
>>> -
>>> /**
>>> * tpm1_get_random() - get random bytes from the TPM's RNG
>>> * @chip: a &struct tpm_chip instance
>>> diff --git a/drivers/char/tpm/tpm1_structs.h b/drivers/char/tpm/tpm1_structs.h
>>> new file mode 100644
>>> index 000000000000..ad21376af5ab
>>> --- /dev/null
>>> +++ b/drivers/char/tpm/tpm1_structs.h
>>> @@ -0,0 +1,97 @@
>>> +/* SPDX-License-Identifier: GPL-2.0-only */
>>> +/*
>>> + * Copyright (C) 2004 IBM Corporation
>>> + * Copyright (C) 2015 Intel Corporation
>>> + *
>>> + * Authors:
>>> + * Leendert van Doorn <leendert@...son.ibm.com>
>>> + * Dave Safford <safford@...son.ibm.com>
>>> + * Reiner Sailer <sailer@...son.ibm.com>
>>> + * Kylene Hall <kjhall@...ibm.com>
>>> + *
>>> + * Maintained by: <tpmdd-devel@...ts.sourceforge.net>
>>> + *
>>> + * Device driver for TCG/TCPA TPM (trusted platform module).
>>> + * Specifications at https://urldefense.com/v3/__http://www.trustedcomputinggroup.org__;!!ACWV5N9M2RV99hQ!OivTxO-R0nvBJYXIV-T0n0fq0wY64MTgdaecPzAauxrFkRxTcQ6CuBEcXmZZYY1KfZlqmfLTZBbPaEr7Y8k7HW5iz_w$ + */
>>> +
>>> +#ifndef __TPM1_STRUCTS_H__
>>> +#define __TPM1_STRUCTS_H__
>>> +
>>> +struct stclear_flags_t {
>>> + __be16 tag;
>>> + u8 deactivated;
>>> + u8 disableForceClear;
>>> + u8 physicalPresence;
>>> + u8 physicalPresenceLock;
>>> + u8 bGlobalLock;
>>> +} __packed;
>>
>>
>> Don't retain alignment.
>>
>
> Okay.
I wanted to clarify what you are asking here. Do you mean to just not pack the structs? It seems some of these structs define the responses from the TPM chip and I would assume they are packed this way.
>
>>> +
>>> +struct tpm1_version {
>>> + u8 major;
>>> + u8 minor;
>>> + u8 rev_major;
>>> + u8 rev_minor;
>>> +} __packed;
>>> +
>>> +struct tpm1_version2 {
>>> + __be16 tag;
>>> + struct tpm1_version version;
>>> +} __packed;
>>> +
>>> +struct timeout_t {
>>> + __be32 a;
>>> + __be32 b;
>>> + __be32 c;
>>> + __be32 d;
>>> +} __packed;
>>> +
>>> +struct duration_t {
>>> + __be32 tpm_short;
>>> + __be32 tpm_medium;
>>> + __be32 tpm_long;
>>> +} __packed;
>>> +
>>> +struct permanent_flags_t {
>>> + __be16 tag;
>>> + u8 disable;
>>> + u8 ownership;
>>> + u8 deactivated;
>>> + u8 readPubek;
>>> + u8 disableOwnerClear;
>>> + u8 allowMaintenance;
>>> + u8 physicalPresenceLifetimeLock;
>>> + u8 physicalPresenceHWEnable;
>>> + u8 physicalPresenceCMDEnable;
>>> + u8 CEKPUsed;
>>> + u8 TPMpost;
>>> + u8 TPMpostLock;
>>> + u8 FIPS;
>>> + u8 operator;
>>> + u8 enableRevokeEK;
>>> + u8 nvLocked;
>>> + u8 readSRKPub;
>>> + u8 tpmEstablished;
>>> + u8 maintenanceDone;
>>> + u8 disableFullDALogicInfo;
>>> +} __packed;
>>> +
>>> +/* Gather all capabilities related information info one type */
>>> +typedef union {
>>> + struct permanent_flags_t perm_flags;
>>> + struct stclear_flags_t stclear_flags;
>>> + __u8 owned;
>>> + __be32 num_pcrs;
>>> + struct tpm1_version version1;
>>> + struct tpm1_version2 version2;
>>> + __be32 manufacturer_id;
>>> + struct timeout_t timeout;
>>> + struct duration_t duration;
>>> +} cap_t;
>>
>> Don't retain alignment here.
Again to clarify, do you mean none of these structs should be packed (and thus the union won't be either)?
Thanks
Ross
>>
>
> Okay.
>
>>> +
>>> +struct tpm1_get_random_out {
>>> + __be32 rng_data_len;
>>> + u8 rng_data[TPM_MAX_RNG_DATA];
>>> +} __packed;
>>> +
>>> +#endif
>>> diff --git a/include/linux/tpm1.h b/include/linux/tpm1.h
>>> index 54c6c211eb9e..5fad94ac8d15 100644
>>> --- a/include/linux/tpm1.h
>>> +++ b/include/linux/tpm1.h
>>> @@ -47,7 +47,39 @@ enum tpm_command_ordinals {
>>> TPM_ORD_UNSEAL = 24,
>>> };
>>> -/* Other constants */
>>> +enum tpm_capabilities {
>>> + TPM_CAP_FLAG = 4,
>>> + TPM_CAP_PROP = 5,
>>> + TPM_CAP_VERSION_1_1 = 0x06,
>>> + TPM_CAP_VERSION_1_2 = 0x1A,
>>> +};
>>> +
>>> +enum tpm_sub_capabilities {
>>> + TPM_CAP_PROP_PCR = 0x101,
>>> + TPM_CAP_PROP_MANUFACTURER = 0x103,
>>> + TPM_CAP_FLAG_PERM = 0x108,
>>> + TPM_CAP_FLAG_VOL = 0x109,
>>> + TPM_CAP_PROP_OWNER = 0x111,
>>> + TPM_CAP_PROP_TIS_TIMEOUT = 0x115,
>>> + TPM_CAP_PROP_TIS_DURATION = 0x120,
>>> +};
>>> +
>>> +/* Return Codes */
>>> +enum tpm_return_codes {
>>> + TPM_BASE_MASK = 0,
>>> + TPM_NON_FATAL_MASK = 0x00000800,
>>> + TPM_SUCCESS = TPM_BASE_MASK + 0,
>>> + TPM_ERR_DEACTIVATED = TPM_BASE_MASK + 6,
>>> + TPM_ERR_DISABLED = TPM_BASE_MASK + 7,
>>> + TPM_ERR_FAIL = TPM_BASE_MASK + 9,
>>> + TPM_ERR_FAILEDSELFTEST = TPM_BASE_MASK + 28,
>>> + TPM_ERR_INVALID_POSTINIT = TPM_BASE_MASK + 38,
>>> + TPM_ERR_INVALID_FAMILY = TPM_BASE_MASK + 55,
>>> + TPM_WARN_RETRY = TPM_BASE_MASK + TPM_NON_FATAL_MASK + 0,
>>> + TPM_WARN_DOING_SELFTEST = TPM_BASE_MASK + TPM_NON_FATAL_MASK + 2,
>>> +};
>>> +
>>> +/* Misc. constants */
>>
>> These constants should be relocated in a separate patch.
>>
>
> Okay.
>
>>> #define SRKHANDLE 0x40000000
>>> #define TPM_NONCE_SIZE 20
>>> #define TPM_ST_CLEAR 1
>>> --
>>> 2.43.7
>>>
>>
>> BR, Jarkko
>
Powered by blists - more mailing lists