* [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies @ 2025-09-29 19:48 Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 01/10] tpm: Cap the number of PCR banks Jarkko Sakkinen ` (10 more replies) 0 siblings, 11 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list Overview ======== Decouple TPM driver features relevant for Trenchboot and make tpm-buf robust and decoupled entity from the rest of driver. By doing this, code can be easily linked to the early boot code. Backlog ======= Parts of tpm_tis should separated and decouple from the driver code so that the slices of code can be compiled into early boot code. Since by other means the series already has most of the gaps filled it's better to resolve this issue before landing the series. v3: - I think 6.19 is a better goal for this and thus expanded the series to be a generic Trenchboot enablers series. This version also consolidates my two separate ongoing series. v2: - While including fixes from v1, this patch set has a refocus in order to do minimal changes to make code base more compatible Trenchboot. Jarkko Sakkinen (10): tpm: Cap the number of PCR banks tpm: Use -EPERM as fallback error code in tpm_ret_to_err KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth tpm2-sessions: Umask tpm_buf_append_hmac_session() KEYS: trusted: Open code tpm2_buf_append() tpm-buf: check for corruption in tpm_buf_append_handle() tpm-buf: Remove chip parameter from tpm_buf_append_handle tpm-buf: Build PCR extend commands tpm-buf: Enable managed and stack allocations. drivers/char/tpm/tpm-buf.c | 208 +++++++++++---- drivers/char/tpm/tpm-chip.c | 13 +- drivers/char/tpm/tpm-sysfs.c | 20 +- drivers/char/tpm/tpm.h | 2 - drivers/char/tpm/tpm1-cmd.c | 179 +++++-------- drivers/char/tpm/tpm2-cmd.c | 307 ++++++++++------------ drivers/char/tpm/tpm2-sessions.c | 128 +++++---- drivers/char/tpm/tpm2-space.c | 44 ++-- drivers/char/tpm/tpm_vtpm_proxy.c | 30 +-- include/linux/tpm.h | 79 +++--- include/linux/tpm_command.h | 5 +- security/keys/trusted-keys/trusted_tpm1.c | 34 ++- security/keys/trusted-keys/trusted_tpm2.c | 237 +++++++---------- 13 files changed, 621 insertions(+), 665 deletions(-) -- 2.39.5 ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 01/10] tpm: Cap the number of PCR banks 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 11:09 ` Jonathan McDowell 2025-09-29 19:48 ` [PATCH v3 02/10] tpm: Use -EPERM as fallback error code in tpm_ret_to_err Jarkko Sakkinen ` (9 subsequent siblings) 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Roberto Sassu, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> tpm2_get_pcr_allocation() does not cap any upper limit for the number of banks. Cap the limit to four banks so that out of bounds values coming from external I/O cause on only limited harm. Cc: Roberto Sassu <roberto.sassu@huawei.com> Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array") Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - Wrote a more clear commit message. - Fixed pr_err() message. v2: - A new patch. --- drivers/char/tpm/tpm-chip.c | 13 +++++++++---- drivers/char/tpm/tpm.h | 1 - drivers/char/tpm/tpm1-cmd.c | 25 ------------------------- drivers/char/tpm/tpm2-cmd.c | 8 +++----- include/linux/tpm.h | 18 ++++++++---------- 5 files changed, 20 insertions(+), 45 deletions(-) diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c index 687f6d8cd601..9a6538f76f50 100644 --- a/drivers/char/tpm/tpm-chip.c +++ b/drivers/char/tpm/tpm-chip.c @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip) static int tpm_get_pcr_allocation(struct tpm_chip *chip) { - int rc; + int rc = 0; if (tpm_is_firmware_upgrade(chip)) return 0; - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ? - tpm2_get_pcr_allocation(chip) : - tpm1_get_pcr_allocation(chip); + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; + chip->nr_allocated_banks = 1; + } else { + rc = tpm2_get_pcr_allocation(chip); + } if (rc > 0) return -ENODEV; diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 57ef8589f5f5..769fa6b00c54 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf); ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap, const char *desc, size_t min_cap_length); int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max); -int tpm1_get_pcr_allocation(struct tpm_chip *chip); unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal); int tpm_pm_suspend(struct device *dev); int tpm_pm_resume(struct device *dev); diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c index cf64c7385105..5c49bdff33de 100644 --- a/drivers/char/tpm/tpm1-cmd.c +++ b/drivers/char/tpm/tpm1-cmd.c @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr) return rc; } - -/** - * tpm1_get_pcr_allocation() - initialize the allocated bank - * @chip: TPM chip to use. - * - * The function initializes the SHA1 allocated bank to extend PCR - * - * Return: - * * 0 on success, - * * < 0 on error. - */ -int tpm1_get_pcr_allocation(struct tpm_chip *chip) -{ - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks), - GFP_KERNEL); - if (!chip->allocated_banks) - return -ENOMEM; - - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; - chip->nr_allocated_banks = 1; - - return 0; -} diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index 7d77f6fbc152..a7cddd4b5626 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) nr_possible_banks = be32_to_cpup( (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]); - - chip->allocated_banks = kcalloc(nr_possible_banks, - sizeof(*chip->allocated_banks), - GFP_KERNEL); - if (!chip->allocated_banks) { + if (nr_possible_banks > TPM2_MAX_BANKS) { + pr_err("tpm: unexpected number of banks: %u > %u", + nr_possible_banks, TPM2_MAX_BANKS); rc = -ENOMEM; goto out; } diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 900c81a2bc41..fc7df87dfb9a 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -27,7 +27,12 @@ #include <crypto/aes.h> #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */ -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE +#define TPM_HEADER_SIZE 10 + +#define TPM2_PLATFORM_PCR 24 +#define TPM2_PCR_SELECT_MIN 3 +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE +#define TPM2_MAX_BANKS 4 struct tpm_chip; struct trusted_key_payload; @@ -69,7 +74,7 @@ enum tpm2_curves { struct tpm_digest { u16 alg_id; - u8 digest[TPM_MAX_DIGEST_SIZE]; + u8 digest[TPM2_MAX_DIGEST_SIZE]; } __packed; struct tpm_bank_info { @@ -190,7 +195,7 @@ struct tpm_chip { unsigned int groups_cnt; u32 nr_allocated_banks; - struct tpm_bank_info *allocated_banks; + struct tpm_bank_info allocated_banks[TPM2_MAX_BANKS]; #ifdef CONFIG_ACPI acpi_handle acpi_dev_handle; char ppi_version[TPM_PPI_VERSION_LEN + 1]; @@ -217,13 +222,6 @@ struct tpm_chip { #endif }; -#define TPM_HEADER_SIZE 10 - -enum tpm2_const { - TPM2_PLATFORM_PCR = 24, - TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8), -}; - enum tpm2_timeouts { TPM2_TIMEOUT_A = 750, TPM2_TIMEOUT_B = 4000, -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 01/10] tpm: Cap the number of PCR banks 2025-09-29 19:48 ` [PATCH v3 01/10] tpm: Cap the number of PCR banks Jarkko Sakkinen @ 2025-09-30 11:09 ` Jonathan McDowell 2025-09-30 12:36 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Jonathan McDowell @ 2025-09-30 11:09 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Roberto Sassu, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Mon, Sep 29, 2025 at 10:48:23PM +0300, Jarkko Sakkinen wrote: > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > tpm2_get_pcr_allocation() does not cap any upper limit for the number of > banks. Cap the limit to four banks so that out of bounds values coming > from external I/O cause on only limited harm. > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array") > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > --- > v3: > - Wrote a more clear commit message. > - Fixed pr_err() message. > v2: > - A new patch. > --- > drivers/char/tpm/tpm-chip.c | 13 +++++++++---- > drivers/char/tpm/tpm.h | 1 - > drivers/char/tpm/tpm1-cmd.c | 25 ------------------------- > drivers/char/tpm/tpm2-cmd.c | 8 +++----- > include/linux/tpm.h | 18 ++++++++---------- > 5 files changed, 20 insertions(+), 45 deletions(-) > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > index 687f6d8cd601..9a6538f76f50 100644 > --- a/drivers/char/tpm/tpm-chip.c > +++ b/drivers/char/tpm/tpm-chip.c > @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip) > > static int tpm_get_pcr_allocation(struct tpm_chip *chip) > { > - int rc; > + int rc = 0; > > if (tpm_is_firmware_upgrade(chip)) > return 0; > > - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ? > - tpm2_get_pcr_allocation(chip) : > - tpm1_get_pcr_allocation(chip); > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { > + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; > + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; > + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; > + chip->nr_allocated_banks = 1; > + } else { > + rc = tpm2_get_pcr_allocation(chip); > + } > > if (rc > 0) > return -ENODEV; > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > index 57ef8589f5f5..769fa6b00c54 100644 > --- a/drivers/char/tpm/tpm.h > +++ b/drivers/char/tpm/tpm.h > @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf); > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap, > const char *desc, size_t min_cap_length); > int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max); > -int tpm1_get_pcr_allocation(struct tpm_chip *chip); > unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal); > int tpm_pm_suspend(struct device *dev); > int tpm_pm_resume(struct device *dev); > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c > index cf64c7385105..5c49bdff33de 100644 > --- a/drivers/char/tpm/tpm1-cmd.c > +++ b/drivers/char/tpm/tpm1-cmd.c > @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr) > > return rc; > } > - > -/** > - * tpm1_get_pcr_allocation() - initialize the allocated bank > - * @chip: TPM chip to use. > - * > - * The function initializes the SHA1 allocated bank to extend PCR > - * > - * Return: > - * * 0 on success, > - * * < 0 on error. > - */ > -int tpm1_get_pcr_allocation(struct tpm_chip *chip) > -{ > - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks), > - GFP_KERNEL); > - if (!chip->allocated_banks) > - return -ENOMEM; > - > - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; > - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; > - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; > - chip->nr_allocated_banks = 1; > - > - return 0; > -} > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index 7d77f6fbc152..a7cddd4b5626 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) > > nr_possible_banks = be32_to_cpup( > (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]); > - > - chip->allocated_banks = kcalloc(nr_possible_banks, > - sizeof(*chip->allocated_banks), > - GFP_KERNEL); > - if (!chip->allocated_banks) { > + if (nr_possible_banks > TPM2_MAX_BANKS) { > + pr_err("tpm: unexpected number of banks: %u > %u", > + nr_possible_banks, TPM2_MAX_BANKS); > rc = -ENOMEM; > goto out; > } > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 900c81a2bc41..fc7df87dfb9a 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -27,7 +27,12 @@ > #include <crypto/aes.h> > > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */ > -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > +#define TPM_HEADER_SIZE 10 > + > +#define TPM2_PLATFORM_PCR 24 > +#define TPM2_PCR_SELECT_MIN 3 By changing this to 3 we lose the fact it's related to TPM2_PLATFORM_PCR - it's the number of bytes required to hold a bitmap with at least TPM2_PLATFORM_PCR entries. Can we at least have a comment about that fact? > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > +#define TPM2_MAX_BANKS 4 Where does this max come from? It matches what I see with swtpm by default (SHA1, SHA2-256, SHA2-384, SHA-512), so I haven't seen anything that exceeds it myself. > struct tpm_chip; > struct trusted_key_payload; > @@ -69,7 +74,7 @@ enum tpm2_curves { > > struct tpm_digest { > u16 alg_id; > - u8 digest[TPM_MAX_DIGEST_SIZE]; > + u8 digest[TPM2_MAX_DIGEST_SIZE]; > } __packed; > > struct tpm_bank_info { > @@ -190,7 +195,7 @@ struct tpm_chip { > unsigned int groups_cnt; > > u32 nr_allocated_banks; > - struct tpm_bank_info *allocated_banks; > + struct tpm_bank_info allocated_banks[TPM2_MAX_BANKS]; > #ifdef CONFIG_ACPI > acpi_handle acpi_dev_handle; > char ppi_version[TPM_PPI_VERSION_LEN + 1]; > @@ -217,13 +222,6 @@ struct tpm_chip { > #endif > }; > > -#define TPM_HEADER_SIZE 10 > - > -enum tpm2_const { > - TPM2_PLATFORM_PCR = 24, > - TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8), > -}; > - > enum tpm2_timeouts { > TPM2_TIMEOUT_A = 750, > TPM2_TIMEOUT_B = 4000, > -- > 2.39.5 > > J. -- "How the f**k did you work that out?" -- Pythagoras ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 01/10] tpm: Cap the number of PCR banks 2025-09-30 11:09 ` Jonathan McDowell @ 2025-09-30 12:36 ` Jarkko Sakkinen 2025-09-30 14:17 ` James Bottomley 0 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-30 12:36 UTC (permalink / raw) To: Jonathan McDowell Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Roberto Sassu, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 12:09:15PM +0100, Jonathan McDowell wrote: > On Mon, Sep 29, 2025 at 10:48:23PM +0300, Jarkko Sakkinen wrote: > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > > > tpm2_get_pcr_allocation() does not cap any upper limit for the number of > > banks. Cap the limit to four banks so that out of bounds values coming > > from external I/O cause on only limited harm. > > > > Cc: Roberto Sassu <roberto.sassu@huawei.com> > > Fixes: bcfff8384f6c ("tpm: dynamically allocate the allocated_banks array") > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > --- > > v3: > > - Wrote a more clear commit message. > > - Fixed pr_err() message. > > v2: > > - A new patch. > > --- > > drivers/char/tpm/tpm-chip.c | 13 +++++++++---- > > drivers/char/tpm/tpm.h | 1 - > > drivers/char/tpm/tpm1-cmd.c | 25 ------------------------- > > drivers/char/tpm/tpm2-cmd.c | 8 +++----- > > include/linux/tpm.h | 18 ++++++++---------- > > 5 files changed, 20 insertions(+), 45 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c > > index 687f6d8cd601..9a6538f76f50 100644 > > --- a/drivers/char/tpm/tpm-chip.c > > +++ b/drivers/char/tpm/tpm-chip.c > > @@ -559,14 +559,19 @@ static int tpm_add_hwrng(struct tpm_chip *chip) > > > > static int tpm_get_pcr_allocation(struct tpm_chip *chip) > > { > > - int rc; > > + int rc = 0; > > > > if (tpm_is_firmware_upgrade(chip)) > > return 0; > > > > - rc = (chip->flags & TPM_CHIP_FLAG_TPM2) ? > > - tpm2_get_pcr_allocation(chip) : > > - tpm1_get_pcr_allocation(chip); > > + if (!(chip->flags & TPM_CHIP_FLAG_TPM2)) { > > + chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; > > + chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; > > + chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; > > + chip->nr_allocated_banks = 1; > > + } else { > > + rc = tpm2_get_pcr_allocation(chip); > > + } > > > > if (rc > 0) > > return -ENODEV; > > diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h > > index 57ef8589f5f5..769fa6b00c54 100644 > > --- a/drivers/char/tpm/tpm.h > > +++ b/drivers/char/tpm/tpm.h > > @@ -252,7 +252,6 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf); > > ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap, > > const char *desc, size_t min_cap_length); > > int tpm1_get_random(struct tpm_chip *chip, u8 *out, size_t max); > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip); > > unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal); > > int tpm_pm_suspend(struct device *dev); > > int tpm_pm_resume(struct device *dev); > > diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c > > index cf64c7385105..5c49bdff33de 100644 > > --- a/drivers/char/tpm/tpm1-cmd.c > > +++ b/drivers/char/tpm/tpm1-cmd.c > > @@ -786,28 +786,3 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr) > > > > return rc; > > } > > - > > -/** > > - * tpm1_get_pcr_allocation() - initialize the allocated bank > > - * @chip: TPM chip to use. > > - * > > - * The function initializes the SHA1 allocated bank to extend PCR > > - * > > - * Return: > > - * * 0 on success, > > - * * < 0 on error. > > - */ > > -int tpm1_get_pcr_allocation(struct tpm_chip *chip) > > -{ > > - chip->allocated_banks = kcalloc(1, sizeof(*chip->allocated_banks), > > - GFP_KERNEL); > > - if (!chip->allocated_banks) > > - return -ENOMEM; > > - > > - chip->allocated_banks[0].alg_id = TPM_ALG_SHA1; > > - chip->allocated_banks[0].digest_size = hash_digest_size[HASH_ALGO_SHA1]; > > - chip->allocated_banks[0].crypto_id = HASH_ALGO_SHA1; > > - chip->nr_allocated_banks = 1; > > - > > - return 0; > > -} > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > > index 7d77f6fbc152..a7cddd4b5626 100644 > > --- a/drivers/char/tpm/tpm2-cmd.c > > +++ b/drivers/char/tpm/tpm2-cmd.c > > @@ -538,11 +538,9 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) > > > > nr_possible_banks = be32_to_cpup( > > (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]); > > - > > - chip->allocated_banks = kcalloc(nr_possible_banks, > > - sizeof(*chip->allocated_banks), > > - GFP_KERNEL); > > - if (!chip->allocated_banks) { > > + if (nr_possible_banks > TPM2_MAX_BANKS) { > > + pr_err("tpm: unexpected number of banks: %u > %u", > > + nr_possible_banks, TPM2_MAX_BANKS); > > rc = -ENOMEM; > > goto out; > > } > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > > index 900c81a2bc41..fc7df87dfb9a 100644 > > --- a/include/linux/tpm.h > > +++ b/include/linux/tpm.h > > @@ -27,7 +27,12 @@ > > #include <crypto/aes.h> > > > > #define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */ > > -#define TPM_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > > +#define TPM_HEADER_SIZE 10 > > + > > +#define TPM2_PLATFORM_PCR 24 > > +#define TPM2_PCR_SELECT_MIN 3 > > By changing this to 3 we lose the fact it's related to TPM2_PLATFORM_PCR > - it's the number of bytes required to hold a bitmap with at least > TPM2_PLATFORM_PCR entries. Can we at least have a comment about that > fact? I'll revert this as it is essentially a spurious change. > > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > > +#define TPM2_MAX_BANKS 4 > > Where does this max come from? It matches what I see with swtpm by > default (SHA1, SHA2-256, SHA2-384, SHA-512), so I haven't seen anything > that exceeds it myself. I've never seen hardware TPM that would have more than one or two banks. We can double it to leave some room. This was tested with swtpm defaults. > > > struct tpm_chip; > > struct trusted_key_payload; > > @@ -69,7 +74,7 @@ enum tpm2_curves { > > > > struct tpm_digest { > > u16 alg_id; > > - u8 digest[TPM_MAX_DIGEST_SIZE]; > > + u8 digest[TPM2_MAX_DIGEST_SIZE]; > > } __packed; > > > > struct tpm_bank_info { > > @@ -190,7 +195,7 @@ struct tpm_chip { > > unsigned int groups_cnt; > > > > u32 nr_allocated_banks; > > - struct tpm_bank_info *allocated_banks; > > + struct tpm_bank_info allocated_banks[TPM2_MAX_BANKS]; > > #ifdef CONFIG_ACPI > > acpi_handle acpi_dev_handle; > > char ppi_version[TPM_PPI_VERSION_LEN + 1]; > > @@ -217,13 +222,6 @@ struct tpm_chip { > > #endif > > }; > > > > -#define TPM_HEADER_SIZE 10 > > - > > -enum tpm2_const { > > - TPM2_PLATFORM_PCR = 24, > > - TPM2_PCR_SELECT_MIN = ((TPM2_PLATFORM_PCR + 7) / 8), > > -}; > > - > > enum tpm2_timeouts { > > TPM2_TIMEOUT_A = 750, > > TPM2_TIMEOUT_B = 4000, > > -- > > 2.39.5 > > > > > > J. > > -- > "How the f**k did you work that out?" -- Pythagoras BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 01/10] tpm: Cap the number of PCR banks 2025-09-30 12:36 ` Jarkko Sakkinen @ 2025-09-30 14:17 ` James Bottomley 2025-10-01 11:16 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: James Bottomley @ 2025-09-30 14:17 UTC (permalink / raw) To: Jarkko Sakkinen, Jonathan McDowell Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Roberto Sassu, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, 2025-09-30 at 15:36 +0300, Jarkko Sakkinen wrote: > On Tue, Sep 30, 2025 at 12:09:15PM +0100, Jonathan McDowell wrote: > > On Mon, Sep 29, 2025 at 10:48:23PM +0300, Jarkko Sakkinen wrote: [...] > > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > > > +#define TPM2_MAX_BANKS 4 > > > > Where does this max come from? It matches what I see with swtpm by > > default (SHA1, SHA2-256, SHA2-384, SHA-512), so I haven't seen > > anything that exceeds it myself. > > I've never seen hardware TPM that would have more than one or two > banks. We can double it to leave some room. This was tested with > swtpm defaults. I've got a hardware TPM that comes with 3 banks by default (it's a chinese one which has sha1 sha256 and sm2). swtpm isn't a good indicator because it's default allocation is rather pejorative (it disables sha1 whereas most field TPMs don't). However, if you look at how the reference implementation works, the user is allowed to define any number of banks they want, up to the number of supported hashes. The only limitation being there can't be >1 bank for the same hash. Field TPM implementations are allowed to constrain this, but most don't. The question you should be asking here is not how many banks does a particular implementation allow by default, but what's the maximum number a user could configure. Regards, James ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 01/10] tpm: Cap the number of PCR banks 2025-09-30 14:17 ` James Bottomley @ 2025-10-01 11:16 ` Jarkko Sakkinen 2025-10-01 12:52 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-10-01 11:16 UTC (permalink / raw) To: James Bottomley Cc: Jonathan McDowell, linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Roberto Sassu, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 10:17:22AM -0400, James Bottomley wrote: > On Tue, 2025-09-30 at 15:36 +0300, Jarkko Sakkinen wrote: > > On Tue, Sep 30, 2025 at 12:09:15PM +0100, Jonathan McDowell wrote: > > > On Mon, Sep 29, 2025 at 10:48:23PM +0300, Jarkko Sakkinen wrote: > [...] > > > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > > > > +#define TPM2_MAX_BANKS 4 > > > > > > Where does this max come from? It matches what I see with swtpm by > > > default (SHA1, SHA2-256, SHA2-384, SHA-512), so I haven't seen > > > anything that exceeds it myself. > > > > I've never seen hardware TPM that would have more than one or two > > banks. We can double it to leave some room. This was tested with > > swtpm defaults. > > I've got a hardware TPM that comes with 3 banks by default (it's a > chinese one which has sha1 sha256 and sm2). swtpm isn't a good > indicator because it's default allocation is rather pejorative (it > disables sha1 whereas most field TPMs don't). > > However, if you look at how the reference implementation works, the > user is allowed to define any number of banks they want, up to the > number of supported hashes. The only limitation being there can't be > >1 bank for the same hash. Field TPM implementations are allowed to > constrain this, but most don't. The question you should be asking > here is not how many banks does a particular implementation allow by > default, but what's the maximum number a user could configure. It needs some compilation time cap as the value comes from external device. If someone hits to that value, then it needs to be increased but as unconstrained it's a bug. > Regards, > > James > BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 01/10] tpm: Cap the number of PCR banks 2025-10-01 11:16 ` Jarkko Sakkinen @ 2025-10-01 12:52 ` Jarkko Sakkinen 0 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-10-01 12:52 UTC (permalink / raw) To: James Bottomley Cc: Jonathan McDowell, linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Roberto Sassu, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Wed, Oct 01, 2025 at 02:16:04PM +0300, Jarkko Sakkinen wrote: > On Tue, Sep 30, 2025 at 10:17:22AM -0400, James Bottomley wrote: > > On Tue, 2025-09-30 at 15:36 +0300, Jarkko Sakkinen wrote: > > > On Tue, Sep 30, 2025 at 12:09:15PM +0100, Jonathan McDowell wrote: > > > > On Mon, Sep 29, 2025 at 10:48:23PM +0300, Jarkko Sakkinen wrote: > > [...] > > > > > +#define TPM2_MAX_DIGEST_SIZE SHA512_DIGEST_SIZE > > > > > +#define TPM2_MAX_BANKS 4 > > > > > > > > Where does this max come from? It matches what I see with swtpm by > > > > default (SHA1, SHA2-256, SHA2-384, SHA-512), so I haven't seen > > > > anything that exceeds it myself. > > > > > > I've never seen hardware TPM that would have more than one or two > > > banks. We can double it to leave some room. This was tested with > > > swtpm defaults. > > > > I've got a hardware TPM that comes with 3 banks by default (it's a > > chinese one which has sha1 sha256 and sm2). swtpm isn't a good > > indicator because it's default allocation is rather pejorative (it > > disables sha1 whereas most field TPMs don't). > > > > However, if you look at how the reference implementation works, the > > user is allowed to define any number of banks they want, up to the > > number of supported hashes. The only limitation being there can't be > > >1 bank for the same hash. Field TPM implementations are allowed to > > constrain this, but most don't. The question you should be asking > > here is not how many banks does a particular implementation allow by > > default, but what's the maximum number a user could configure. > > It needs some compilation time cap as the value comes from external > device. If someone hits to that value, then it needs to be increased > but as unconstrained it's a bug. Maximum eight banks should be spacy enough for the time being (and for the foreseeable future). BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 02/10] tpm: Use -EPERM as fallback error code in tpm_ret_to_err 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 01/10] tpm: Cap the number of PCR banks Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 12:11 ` Stefano Garzarella 2025-09-29 19:48 ` [PATCH v3 03/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 Jarkko Sakkinen ` (8 subsequent siblings) 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Using -EFAULT as the tpm_ret_to_err() fallback error code causes makes it incompatible on how trusted keys transmute TPM return codes. Change the fallback as -EPERM in order to gain compatibility with trusted keys. In addition, map TPM_RC_HASH to -EINVAL in order to be compatible with tpm2_seal_trusted() return values. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - Removed fixes tag as it hardly categorizes as a bug fix. v2: - Split trusted_tpm2 change to a separate patch. --- include/linux/tpm.h | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/include/linux/tpm.h b/include/linux/tpm.h index fc7df87dfb9a..51846317d662 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -453,8 +453,10 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret) return 0; case TPM2_RC_SESSION_MEMORY: return -ENOMEM; + case TPM2_RC_HASH: + return -EINVAL; default: - return -EFAULT; + return -EPERM; } } -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 02/10] tpm: Use -EPERM as fallback error code in tpm_ret_to_err 2025-09-29 19:48 ` [PATCH v3 02/10] tpm: Use -EPERM as fallback error code in tpm_ret_to_err Jarkko Sakkinen @ 2025-09-30 12:11 ` Stefano Garzarella 2025-09-30 12:37 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Stefano Garzarella @ 2025-09-30 12:11 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Mon, Sep 29, 2025 at 10:48:24PM +0300, Jarkko Sakkinen wrote: >From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > >Using -EFAULT as the tpm_ret_to_err() fallback error code causes makes it >incompatible on how trusted keys transmute TPM return codes. > >Change the fallback as -EPERM in order to gain compatibility with trusted >keys. In addition, map TPM_RC_HASH to -EINVAL in order to be compatible >with tpm2_seal_trusted() return values. > >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> >--- >v3: >- Removed fixes tag as it hardly categorizes as a bug fix. >v2: >- Split trusted_tpm2 change to a separate patch. >--- > include/linux/tpm.h | 4 +++- > 1 file changed, 3 insertions(+), 1 deletion(-) LGTM now! Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> > >diff --git a/include/linux/tpm.h b/include/linux/tpm.h >index fc7df87dfb9a..51846317d662 100644 >--- a/include/linux/tpm.h >+++ b/include/linux/tpm.h >@@ -453,8 +453,10 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret) > return 0; > case TPM2_RC_SESSION_MEMORY: > return -ENOMEM; >+ case TPM2_RC_HASH: >+ return -EINVAL; > default: >- return -EFAULT; >+ return -EPERM; > } > } > >-- >2.39.5 > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 02/10] tpm: Use -EPERM as fallback error code in tpm_ret_to_err 2025-09-30 12:11 ` Stefano Garzarella @ 2025-09-30 12:37 ` Jarkko Sakkinen 0 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-30 12:37 UTC (permalink / raw) To: Stefano Garzarella Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 02:11:23PM +0200, Stefano Garzarella wrote: > On Mon, Sep 29, 2025 at 10:48:24PM +0300, Jarkko Sakkinen wrote: > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > > > Using -EFAULT as the tpm_ret_to_err() fallback error code causes makes it > > incompatible on how trusted keys transmute TPM return codes. > > > > Change the fallback as -EPERM in order to gain compatibility with trusted > > keys. In addition, map TPM_RC_HASH to -EINVAL in order to be compatible > > with tpm2_seal_trusted() return values. > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > --- > > v3: > > - Removed fixes tag as it hardly categorizes as a bug fix. > > v2: > > - Split trusted_tpm2 change to a separate patch. > > --- > > include/linux/tpm.h | 4 +++- > > 1 file changed, 3 insertions(+), 1 deletion(-) > > LGTM now! > > Reviewed-by: Stefano Garzarella <sgarzare@redhat.com> Thank you. > > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > > index fc7df87dfb9a..51846317d662 100644 > > --- a/include/linux/tpm.h > > +++ b/include/linux/tpm.h > > @@ -453,8 +453,10 @@ static inline ssize_t tpm_ret_to_err(ssize_t ret) > > return 0; > > case TPM2_RC_SESSION_MEMORY: > > return -ENOMEM; > > + case TPM2_RC_HASH: > > + return -EINVAL; > > default: > > - return -EFAULT; > > + return -EPERM; > > } > > } > > > > -- > > 2.39.5 > > > > > BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 03/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 01/10] tpm: Cap the number of PCR banks Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 02/10] tpm: Use -EPERM as fallback error code in tpm_ret_to_err Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 12:12 ` Stefano Garzarella 2025-09-29 19:48 ` [PATCH v3 04/10] tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth Jarkko Sakkinen ` (7 subsequent siblings) 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, David Howells, Jarkko Sakkinen, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Use tpm_ret_to_err() to transmute TPM return codes in trusted_tpm2. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - No changes. v2: - New patch split out from the fix. --- security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++----------------- 1 file changed, 7 insertions(+), 19 deletions(-) diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index 024be262702f..e165b117bbca 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip, } blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len); + if (blob_len < 0) + rc = blob_len; out: tpm_buf_destroy(&sized); tpm_buf_destroy(&buf); - if (rc > 0) { - if (tpm2_rc_value(rc) == TPM2_RC_HASH) - rc = -EINVAL; - else - rc = -EPERM; - } - if (blob_len < 0) - rc = blob_len; - else + if (!rc) payload->blob_len = blob_len; out_put: tpm_put_ops(chip); - return rc; + return tpm_ret_to_err(rc); } /** @@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, kfree(blob); tpm_buf_destroy(&buf); - if (rc > 0) - rc = -EPERM; - - return rc; + return tpm_ret_to_err(rc); } /** @@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, tpm_buf_fill_hmac_session(chip, &buf); rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing"); rc = tpm_buf_check_hmac_response(chip, &buf, rc); - if (rc > 0) - rc = -EPERM; if (!rc) { data_len = be16_to_cpup( @@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, out: tpm_buf_destroy(&buf); - return rc; + return tpm_ret_to_err(rc); } /** @@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip, out: tpm_put_ops(chip); - - return rc; + return tpm_ret_to_err(rc); } -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 03/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 2025-09-29 19:48 ` [PATCH v3 03/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 Jarkko Sakkinen @ 2025-09-30 12:12 ` Stefano Garzarella 2025-09-30 12:39 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Stefano Garzarella @ 2025-09-30 12:12 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list On Mon, Sep 29, 2025 at 10:48:25PM +0300, Jarkko Sakkinen wrote: >From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > >Use tpm_ret_to_err() to transmute TPM return codes in trusted_tpm2. > >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> >--- >v3: >- No changes. >v2: >- New patch split out from the fix. >--- > security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++----------------- > 1 file changed, 7 insertions(+), 19 deletions(-) Nice clean up! Acked-by: Stefano Garzarella <sgarzare@redhat.com> > >diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c >index 024be262702f..e165b117bbca 100644 >--- a/security/keys/trusted-keys/trusted_tpm2.c >+++ b/security/keys/trusted-keys/trusted_tpm2.c >@@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > } > > blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len); >+ if (blob_len < 0) >+ rc = blob_len; > > out: > tpm_buf_destroy(&sized); > tpm_buf_destroy(&buf); > >- if (rc > 0) { >- if (tpm2_rc_value(rc) == TPM2_RC_HASH) >- rc = -EINVAL; >- else >- rc = -EPERM; >- } >- if (blob_len < 0) >- rc = blob_len; >- else >+ if (!rc) > payload->blob_len = blob_len; > > out_put: > tpm_put_ops(chip); >- return rc; >+ return tpm_ret_to_err(rc); > } > > /** >@@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, > kfree(blob); > tpm_buf_destroy(&buf); > >- if (rc > 0) >- rc = -EPERM; >- >- return rc; >+ return tpm_ret_to_err(rc); > } > > /** >@@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > tpm_buf_fill_hmac_session(chip, &buf); > rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing"); > rc = tpm_buf_check_hmac_response(chip, &buf, rc); >- if (rc > 0) >- rc = -EPERM; > > if (!rc) { > data_len = be16_to_cpup( >@@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > > out: > tpm_buf_destroy(&buf); >- return rc; >+ return tpm_ret_to_err(rc); > } > > /** >@@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip, > > out: > tpm_put_ops(chip); >- >- return rc; >+ return tpm_ret_to_err(rc); > } >-- >2.39.5 > > ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 03/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 2025-09-30 12:12 ` Stefano Garzarella @ 2025-09-30 12:39 ` Jarkko Sakkinen 0 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-30 12:39 UTC (permalink / raw) To: Stefano Garzarella Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list On Tue, Sep 30, 2025 at 02:12:35PM +0200, Stefano Garzarella wrote: > On Mon, Sep 29, 2025 at 10:48:25PM +0300, Jarkko Sakkinen wrote: > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > > > Use tpm_ret_to_err() to transmute TPM return codes in trusted_tpm2. > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > --- > > v3: > > - No changes. > > v2: > > - New patch split out from the fix. > > --- > > security/keys/trusted-keys/trusted_tpm2.c | 26 ++++++----------------- > > 1 file changed, 7 insertions(+), 19 deletions(-) > > Nice clean up! > > Acked-by: Stefano Garzarella <sgarzare@redhat.com> Thank you. > > > > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > > index 024be262702f..e165b117bbca 100644 > > --- a/security/keys/trusted-keys/trusted_tpm2.c > > +++ b/security/keys/trusted-keys/trusted_tpm2.c > > @@ -348,25 +348,19 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > > } > > > > blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len); > > + if (blob_len < 0) > > + rc = blob_len; > > > > out: > > tpm_buf_destroy(&sized); > > tpm_buf_destroy(&buf); > > > > - if (rc > 0) { > > - if (tpm2_rc_value(rc) == TPM2_RC_HASH) > > - rc = -EINVAL; > > - else > > - rc = -EPERM; > > - } > > - if (blob_len < 0) > > - rc = blob_len; > > - else > > + if (!rc) > > payload->blob_len = blob_len; > > > > out_put: > > tpm_put_ops(chip); > > - return rc; > > + return tpm_ret_to_err(rc); > > } > > > > /** > > @@ -468,10 +462,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, > > kfree(blob); > > tpm_buf_destroy(&buf); > > > > - if (rc > 0) > > - rc = -EPERM; > > - > > - return rc; > > + return tpm_ret_to_err(rc); > > } > > > > /** > > @@ -534,8 +525,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > > tpm_buf_fill_hmac_session(chip, &buf); > > rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing"); > > rc = tpm_buf_check_hmac_response(chip, &buf, rc); > > - if (rc > 0) > > - rc = -EPERM; > > > > if (!rc) { > > data_len = be16_to_cpup( > > @@ -568,7 +557,7 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > > > > out: > > tpm_buf_destroy(&buf); > > - return rc; > > + return tpm_ret_to_err(rc); > > } > > > > /** > > @@ -600,6 +589,5 @@ int tpm2_unseal_trusted(struct tpm_chip *chip, > > > > out: > > tpm_put_ops(chip); > > - > > - return rc; > > + return tpm_ret_to_err(rc); > > } > > -- > > 2.39.5 > > > > > BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 04/10] tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (2 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 03/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 11:10 ` Jonathan McDowell 2025-09-29 19:48 ` [PATCH v3 05/10] tpm2-sessions: Umask tpm_buf_append_hmac_session() Jarkko Sakkinen ` (6 subsequent siblings) 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, Roberto Sassu, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> In a previous bug fix, 'attributes' was added by mistake to tpm_buf_append_auth(). Remove the parameter. Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection") Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - No changes v2: - Uncorrupt the patch. --- drivers/char/tpm/tpm2-cmd.c | 2 +- drivers/char/tpm/tpm2-sessions.c | 5 ++--- include/linux/tpm.h | 2 +- 3 files changed, 4 insertions(+), 5 deletions(-) diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index a7cddd4b5626..86b1a4d859b9 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -191,7 +191,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0); } else { tpm_buf_append_handle(chip, &buf, pcr_idx); - tpm_buf_append_auth(chip, &buf, 0, NULL, 0); + tpm_buf_append_auth(chip, &buf, NULL, 0); } tpm_buf_append_u32(&buf, chip->nr_allocated_banks); diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c index 6d03c224e6b2..13f019d1312a 100644 --- a/drivers/char/tpm/tpm2-sessions.c +++ b/drivers/char/tpm/tpm2-sessions.c @@ -266,7 +266,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf, EXPORT_SYMBOL_GPL(tpm_buf_append_name); void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, - u8 attributes, u8 *passphrase, int passphrase_len) + u8 *passphrase, int passphrase_len) { /* offset tells us where the sessions area begins */ int offset = buf->handles * 4 + TPM_HEADER_SIZE; @@ -327,8 +327,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, #endif if (!tpm2_chip_auth(chip)) { - tpm_buf_append_auth(chip, buf, attributes, passphrase, - passphrase_len); + tpm_buf_append_auth(chip, buf, passphrase, passphrase_len); return; } diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 51846317d662..1fa02e18e688 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -531,7 +531,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, u8 attributes, u8 *passphrase, int passphraselen); void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, - u8 attributes, u8 *passphrase, int passphraselen); + u8 *passphrase, int passphraselen); static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip, struct tpm_buf *buf, u8 attributes, -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 04/10] tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth 2025-09-29 19:48 ` [PATCH v3 04/10] tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth Jarkko Sakkinen @ 2025-09-30 11:10 ` Jonathan McDowell 2025-09-30 12:39 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Jonathan McDowell @ 2025-09-30 11:10 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, Roberto Sassu, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Mon, Sep 29, 2025 at 10:48:26PM +0300, Jarkko Sakkinen wrote: > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > In a previous bug fix, 'attributes' was added by mistake to > tpm_buf_append_auth(). Remove the parameter. > > Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection") > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Reviewed-by: Jonathan McDowell <noodles@meta.com> > --- > v3: > - No changes > v2: > - Uncorrupt the patch. > --- > drivers/char/tpm/tpm2-cmd.c | 2 +- > drivers/char/tpm/tpm2-sessions.c | 5 ++--- > include/linux/tpm.h | 2 +- > 3 files changed, 4 insertions(+), 5 deletions(-) > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index a7cddd4b5626..86b1a4d859b9 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -191,7 +191,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, > tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0); > } else { > tpm_buf_append_handle(chip, &buf, pcr_idx); > - tpm_buf_append_auth(chip, &buf, 0, NULL, 0); > + tpm_buf_append_auth(chip, &buf, NULL, 0); > } > > tpm_buf_append_u32(&buf, chip->nr_allocated_banks); > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c > index 6d03c224e6b2..13f019d1312a 100644 > --- a/drivers/char/tpm/tpm2-sessions.c > +++ b/drivers/char/tpm/tpm2-sessions.c > @@ -266,7 +266,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf, > EXPORT_SYMBOL_GPL(tpm_buf_append_name); > > void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, > - u8 attributes, u8 *passphrase, int passphrase_len) > + u8 *passphrase, int passphrase_len) > { > /* offset tells us where the sessions area begins */ > int offset = buf->handles * 4 + TPM_HEADER_SIZE; > @@ -327,8 +327,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, > #endif > > if (!tpm2_chip_auth(chip)) { > - tpm_buf_append_auth(chip, buf, attributes, passphrase, > - passphrase_len); > + tpm_buf_append_auth(chip, buf, passphrase, passphrase_len); > return; > } > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 51846317d662..1fa02e18e688 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -531,7 +531,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, > u8 attributes, u8 *passphrase, > int passphraselen); > void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, > - u8 attributes, u8 *passphrase, int passphraselen); > + u8 *passphrase, int passphraselen); > static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip, > struct tpm_buf *buf, > u8 attributes, > -- > 2.39.5 > > J. -- 101 things you can't have too | .''`. Debian GNU/Linux Developer much of : 17 - Money. | : :' : Happy to accept PGP signed | `. `' or encrypted mail - RSA | `- key on the keyservers. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 04/10] tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth 2025-09-30 11:10 ` Jonathan McDowell @ 2025-09-30 12:39 ` Jarkko Sakkinen 0 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-30 12:39 UTC (permalink / raw) To: Jonathan McDowell Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, Roberto Sassu, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 12:10:41PM +0100, Jonathan McDowell wrote: > On Mon, Sep 29, 2025 at 10:48:26PM +0300, Jarkko Sakkinen wrote: > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > > > In a previous bug fix, 'attributes' was added by mistake to > > tpm_buf_append_auth(). Remove the parameter. > > > > Fixes: 27184f8905ba ("tpm: Opt-in in disable PCR integrity protection") > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > Reviewed-by: Jonathan McDowell <noodles@meta.com> Thank you. > > > --- > > v3: > > - No changes > > v2: > > - Uncorrupt the patch. > > --- > > drivers/char/tpm/tpm2-cmd.c | 2 +- > > drivers/char/tpm/tpm2-sessions.c | 5 ++--- > > include/linux/tpm.h | 2 +- > > 3 files changed, 4 insertions(+), 5 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > > index a7cddd4b5626..86b1a4d859b9 100644 > > --- a/drivers/char/tpm/tpm2-cmd.c > > +++ b/drivers/char/tpm/tpm2-cmd.c > > @@ -191,7 +191,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, > > tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0); > > } else { > > tpm_buf_append_handle(chip, &buf, pcr_idx); > > - tpm_buf_append_auth(chip, &buf, 0, NULL, 0); > > + tpm_buf_append_auth(chip, &buf, NULL, 0); > > } > > > > tpm_buf_append_u32(&buf, chip->nr_allocated_banks); > > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c > > index 6d03c224e6b2..13f019d1312a 100644 > > --- a/drivers/char/tpm/tpm2-sessions.c > > +++ b/drivers/char/tpm/tpm2-sessions.c > > @@ -266,7 +266,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf, > > EXPORT_SYMBOL_GPL(tpm_buf_append_name); > > > > void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, > > - u8 attributes, u8 *passphrase, int passphrase_len) > > + u8 *passphrase, int passphrase_len) > > { > > /* offset tells us where the sessions area begins */ > > int offset = buf->handles * 4 + TPM_HEADER_SIZE; > > @@ -327,8 +327,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, > > #endif > > > > if (!tpm2_chip_auth(chip)) { > > - tpm_buf_append_auth(chip, buf, attributes, passphrase, > > - passphrase_len); > > + tpm_buf_append_auth(chip, buf, passphrase, passphrase_len); > > return; > > } > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > > index 51846317d662..1fa02e18e688 100644 > > --- a/include/linux/tpm.h > > +++ b/include/linux/tpm.h > > @@ -531,7 +531,7 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, > > u8 attributes, u8 *passphrase, > > int passphraselen); > > void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, > > - u8 attributes, u8 *passphrase, int passphraselen); > > + u8 *passphrase, int passphraselen); > > static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip, > > struct tpm_buf *buf, > > u8 attributes, > > -- > > 2.39.5 > > > > > > J. > > -- > 101 things you can't have too | .''`. Debian GNU/Linux Developer > much of : 17 - Money. | : :' : Happy to accept PGP signed > | `. `' or encrypted mail - RSA > | `- key on the keyservers. BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 05/10] tpm2-sessions: Umask tpm_buf_append_hmac_session() 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (3 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 04/10] tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 11:11 ` Jonathan McDowell 2025-09-29 19:48 ` [PATCH v3 06/10] KEYS: trusted: Open code tpm2_buf_append() Jarkko Sakkinen ` (5 subsequent siblings) 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Open code tpm_buf_append_hmac_session_opt() in order to unmask the code paths in the call sites of tpm_buf_append_hmac_session(). Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - No changes. v2: - Uncorrupt the patch. --- drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++--- include/linux/tpm.h | 23 ----------------------- security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++-- 3 files changed, 21 insertions(+), 28 deletions(-) diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index 86b1a4d859b9..c7bfa705ea8f 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -257,9 +257,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) do { tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM); - tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT - | TPM2_SA_CONTINUE_SESSION, - NULL, 0); + if (tpm2_chip_auth(chip)) { + tpm_buf_append_hmac_session(chip, &buf, + TPM2_SA_ENCRYPT | + TPM2_SA_CONTINUE_SESSION, + NULL, 0); + } else { + offset = buf.handles * 4 + TPM_HEADER_SIZE; + head = (struct tpm_header *)buf.data; + if (tpm_buf_length(&buf) == offset) + head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); + } tpm_buf_append_u16(&buf, num_bytes); tpm_buf_fill_hmac_session(chip, &buf); err = tpm_transmit_cmd(chip, &buf, diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 1fa02e18e688..e72e7657faa2 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -532,29 +532,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, int passphraselen); void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, u8 *passphrase, int passphraselen); -static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip, - struct tpm_buf *buf, - u8 attributes, - u8 *passphrase, - int passphraselen) -{ - struct tpm_header *head; - int offset; - - if (tpm2_chip_auth(chip)) { - tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen); - } else { - offset = buf->handles * 4 + TPM_HEADER_SIZE; - head = (struct tpm_header *)buf->data; - - /* - * If the only sessions are optional, the command tag must change to - * TPM2_ST_NO_SESSIONS. - */ - if (tpm_buf_length(buf) == offset) - head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); - } -} #ifdef CONFIG_TCG_TPM2_HMAC diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index e165b117bbca..c414a7006d78 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -482,8 +482,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, struct trusted_key_options *options, u32 blob_handle) { + struct tpm_header *head; struct tpm_buf buf; u16 data_len; + int offset; u8 *data; int rc; @@ -518,8 +520,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, tpm2_buf_append_auth(&buf, options->policyhandle, NULL /* nonce */, 0, 0, options->blobauth, options->blobauth_len); - tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT, - NULL, 0); + if (tpm2_chip_auth(chip)) { + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0); + } else { + offset = buf.handles * 4 + TPM_HEADER_SIZE; + head = (struct tpm_header *)buf.data; + if (tpm_buf_length(&buf) == offset) + head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); + } } tpm_buf_fill_hmac_session(chip, &buf); -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 05/10] tpm2-sessions: Umask tpm_buf_append_hmac_session() 2025-09-29 19:48 ` [PATCH v3 05/10] tpm2-sessions: Umask tpm_buf_append_hmac_session() Jarkko Sakkinen @ 2025-09-30 11:11 ` Jonathan McDowell 2025-09-30 12:41 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Jonathan McDowell @ 2025-09-30 11:11 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Mon, Sep 29, 2025 at 10:48:27PM +0300, Jarkko Sakkinen wrote: > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > Open code tpm_buf_append_hmac_session_opt() in order to unmask the code > paths in the call sites of tpm_buf_append_hmac_session(). > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Reviewed-by: Jonathan McDowell <noodles@meta.com> > --- > v3: > - No changes. > v2: > - Uncorrupt the patch. > --- > drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++--- > include/linux/tpm.h | 23 ----------------------- > security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++-- > 3 files changed, 21 insertions(+), 28 deletions(-) > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index 86b1a4d859b9..c7bfa705ea8f 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -257,9 +257,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) > > do { > tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM); > - tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT > - | TPM2_SA_CONTINUE_SESSION, > - NULL, 0); > + if (tpm2_chip_auth(chip)) { > + tpm_buf_append_hmac_session(chip, &buf, > + TPM2_SA_ENCRYPT | > + TPM2_SA_CONTINUE_SESSION, > + NULL, 0); > + } else { > + offset = buf.handles * 4 + TPM_HEADER_SIZE; > + head = (struct tpm_header *)buf.data; > + if (tpm_buf_length(&buf) == offset) > + head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); > + } > tpm_buf_append_u16(&buf, num_bytes); > tpm_buf_fill_hmac_session(chip, &buf); > err = tpm_transmit_cmd(chip, &buf, > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 1fa02e18e688..e72e7657faa2 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -532,29 +532,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, > int passphraselen); > void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, > u8 *passphrase, int passphraselen); > -static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip, > - struct tpm_buf *buf, > - u8 attributes, > - u8 *passphrase, > - int passphraselen) > -{ > - struct tpm_header *head; > - int offset; > - > - if (tpm2_chip_auth(chip)) { > - tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen); > - } else { > - offset = buf->handles * 4 + TPM_HEADER_SIZE; > - head = (struct tpm_header *)buf->data; > - > - /* > - * If the only sessions are optional, the command tag must change to > - * TPM2_ST_NO_SESSIONS. > - */ > - if (tpm_buf_length(buf) == offset) > - head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); > - } > -} > > #ifdef CONFIG_TCG_TPM2_HMAC > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > index e165b117bbca..c414a7006d78 100644 > --- a/security/keys/trusted-keys/trusted_tpm2.c > +++ b/security/keys/trusted-keys/trusted_tpm2.c > @@ -482,8 +482,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > struct trusted_key_options *options, > u32 blob_handle) > { > + struct tpm_header *head; > struct tpm_buf buf; > u16 data_len; > + int offset; > u8 *data; > int rc; > > @@ -518,8 +520,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > tpm2_buf_append_auth(&buf, options->policyhandle, > NULL /* nonce */, 0, 0, > options->blobauth, options->blobauth_len); > - tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT, > - NULL, 0); > + if (tpm2_chip_auth(chip)) { > + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0); > + } else { > + offset = buf.handles * 4 + TPM_HEADER_SIZE; > + head = (struct tpm_header *)buf.data; > + if (tpm_buf_length(&buf) == offset) > + head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); > + } > } > > tpm_buf_fill_hmac_session(chip, &buf); > -- > 2.39.5 > > J. -- ... I love you the way a bomb loves a crowd. ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 05/10] tpm2-sessions: Umask tpm_buf_append_hmac_session() 2025-09-30 11:11 ` Jonathan McDowell @ 2025-09-30 12:41 ` Jarkko Sakkinen 0 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-30 12:41 UTC (permalink / raw) To: Jonathan McDowell Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 12:11:47PM +0100, Jonathan McDowell wrote: > On Mon, Sep 29, 2025 at 10:48:27PM +0300, Jarkko Sakkinen wrote: > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > > > Open code tpm_buf_append_hmac_session_opt() in order to unmask the code > > paths in the call sites of tpm_buf_append_hmac_session(). > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > Reviewed-by: Jonathan McDowell <noodles@meta.com> Thanks (short summary has a typo tho) > > > --- > > v3: > > - No changes. > > v2: > > - Uncorrupt the patch. > > --- > > drivers/char/tpm/tpm2-cmd.c | 14 +++++++++++--- > > include/linux/tpm.h | 23 ----------------------- > > security/keys/trusted-keys/trusted_tpm2.c | 12 ++++++++++-- > > 3 files changed, 21 insertions(+), 28 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > > index 86b1a4d859b9..c7bfa705ea8f 100644 > > --- a/drivers/char/tpm/tpm2-cmd.c > > +++ b/drivers/char/tpm/tpm2-cmd.c > > @@ -257,9 +257,17 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) > > > > do { > > tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM); > > - tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT > > - | TPM2_SA_CONTINUE_SESSION, > > - NULL, 0); > > + if (tpm2_chip_auth(chip)) { > > + tpm_buf_append_hmac_session(chip, &buf, > > + TPM2_SA_ENCRYPT | > > + TPM2_SA_CONTINUE_SESSION, > > + NULL, 0); > > + } else { > > + offset = buf.handles * 4 + TPM_HEADER_SIZE; > > + head = (struct tpm_header *)buf.data; > > + if (tpm_buf_length(&buf) == offset) > > + head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); > > + } > > tpm_buf_append_u16(&buf, num_bytes); > > tpm_buf_fill_hmac_session(chip, &buf); > > err = tpm_transmit_cmd(chip, &buf, > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > > index 1fa02e18e688..e72e7657faa2 100644 > > --- a/include/linux/tpm.h > > +++ b/include/linux/tpm.h > > @@ -532,29 +532,6 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf, > > int passphraselen); > > void tpm_buf_append_auth(struct tpm_chip *chip, struct tpm_buf *buf, > > u8 *passphrase, int passphraselen); > > -static inline void tpm_buf_append_hmac_session_opt(struct tpm_chip *chip, > > - struct tpm_buf *buf, > > - u8 attributes, > > - u8 *passphrase, > > - int passphraselen) > > -{ > > - struct tpm_header *head; > > - int offset; > > - > > - if (tpm2_chip_auth(chip)) { > > - tpm_buf_append_hmac_session(chip, buf, attributes, passphrase, passphraselen); > > - } else { > > - offset = buf->handles * 4 + TPM_HEADER_SIZE; > > - head = (struct tpm_header *)buf->data; > > - > > - /* > > - * If the only sessions are optional, the command tag must change to > > - * TPM2_ST_NO_SESSIONS. > > - */ > > - if (tpm_buf_length(buf) == offset) > > - head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); > > - } > > -} > > > > #ifdef CONFIG_TCG_TPM2_HMAC > > > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > > index e165b117bbca..c414a7006d78 100644 > > --- a/security/keys/trusted-keys/trusted_tpm2.c > > +++ b/security/keys/trusted-keys/trusted_tpm2.c > > @@ -482,8 +482,10 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > > struct trusted_key_options *options, > > u32 blob_handle) > > { > > + struct tpm_header *head; > > struct tpm_buf buf; > > u16 data_len; > > + int offset; > > u8 *data; > > int rc; > > > > @@ -518,8 +520,14 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, > > tpm2_buf_append_auth(&buf, options->policyhandle, > > NULL /* nonce */, 0, 0, > > options->blobauth, options->blobauth_len); > > - tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT, > > - NULL, 0); > > + if (tpm2_chip_auth(chip)) { > > + tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0); > > + } else { > > + offset = buf.handles * 4 + TPM_HEADER_SIZE; > > + head = (struct tpm_header *)buf.data; > > + if (tpm_buf_length(&buf) == offset) > > + head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); > > + } > > } > > > > tpm_buf_fill_hmac_session(chip, &buf); > > -- > > 2.39.5 > > > > > > J. > > -- > ... I love you the way a bomb loves a crowd. BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 06/10] KEYS: trusted: Open code tpm2_buf_append() 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (4 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 05/10] tpm2-sessions: Umask tpm_buf_append_hmac_session() Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 07/10] tpm-buf: check for corruption in tpm_buf_append_handle() Jarkko Sakkinen ` (4 subsequent siblings) 10 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Jonathan McDowell, David Howells, Jarkko Sakkinen, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> tpm2_buf_append_auth() has only single call site and most of its parameters are redundant. Open code it to the call site. Remove illegit FIXME comment as there is no categorized bug and replace it with more sane comment about implementation (i.e. "non-opionated inline comment"). Reviewed-by: Jonathan McDowell <noodles@earth.li> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> v3: - No changes. v2: - No changes. --- security/keys/trusted-keys/trusted_tpm2.c | 51 ++++------------------- 1 file changed, 9 insertions(+), 42 deletions(-) diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index c414a7006d78..8e3b283a59b2 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -198,36 +198,6 @@ int tpm2_key_priv(void *context, size_t hdrlen, return 0; } -/** - * tpm2_buf_append_auth() - append TPMS_AUTH_COMMAND to the buffer. - * - * @buf: an allocated tpm_buf instance - * @session_handle: session handle - * @nonce: the session nonce, may be NULL if not used - * @nonce_len: the session nonce length, may be 0 if not used - * @attributes: the session attributes - * @hmac: the session HMAC or password, may be NULL if not used - * @hmac_len: the session HMAC or password length, maybe 0 if not used - */ -static void tpm2_buf_append_auth(struct tpm_buf *buf, u32 session_handle, - const u8 *nonce, u16 nonce_len, - u8 attributes, - const u8 *hmac, u16 hmac_len) -{ - tpm_buf_append_u32(buf, 9 + nonce_len + hmac_len); - tpm_buf_append_u32(buf, session_handle); - tpm_buf_append_u16(buf, nonce_len); - - if (nonce && nonce_len) - tpm_buf_append(buf, nonce, nonce_len); - - tpm_buf_append_u8(buf, attributes); - tpm_buf_append_u16(buf, hmac_len); - - if (hmac && hmac_len) - tpm_buf_append(buf, hmac, hmac_len); -} - /** * tpm2_seal_trusted() - seal the payload of a trusted key * @@ -507,19 +477,16 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, options->blobauth_len); } else { /* - * FIXME: The policy session was generated outside the - * kernel so we don't known the nonce and thus can't - * calculate a HMAC on it. Therefore, the user can - * only really use TPM2_PolicyPassword and we must - * send down the plain text password, which could be - * intercepted. We can still encrypt the returned - * key, but that's small comfort since the interposer - * could repeat our actions with the exfiltrated - * password. + * The policy session is generated outside the kernel, and thus + * the password will end up being unencrypted on the bus, as + * HMAC nonce cannot be calculated for it. */ - tpm2_buf_append_auth(&buf, options->policyhandle, - NULL /* nonce */, 0, 0, - options->blobauth, options->blobauth_len); + tpm_buf_append_u32(&buf, 9 + options->blobauth_len); + tpm_buf_append_u32(&buf, options->policyhandle); + tpm_buf_append_u16(&buf, 0); + tpm_buf_append_u8(&buf, 0); + tpm_buf_append_u16(&buf, options->blobauth_len); + tpm_buf_append(&buf, options->blobauth, options->blobauth_len); if (tpm2_chip_auth(chip)) { tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0); } else { -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 07/10] tpm-buf: check for corruption in tpm_buf_append_handle() 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (5 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 06/10] KEYS: trusted: Open code tpm2_buf_append() Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 11:13 ` Jonathan McDowell 2025-09-29 19:48 ` [PATCH v3 08/10] tpm-buf: Remove chip parameter from tpm_buf_append_handle Jarkko Sakkinen ` (3 subsequent siblings) 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Unify TPM_BUF_BOUNDARY_ERROR and TPM_BUF_OVERFLOW into TPM_BUF_INVALID flag because semantically they are identical. Test and set TPM_BUF_INVALID in tpm_buf_append_handle() following the pattern from other functions in tpm-buf.c. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - No changes. v2: - A new patch. --- drivers/char/tpm/tpm-buf.c | 14 ++++++++------ include/linux/tpm.h | 8 +++----- security/keys/trusted-keys/trusted_tpm2.c | 6 +++--- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c index dc882fc9fa9e..69ee77400539 100644 --- a/drivers/char/tpm/tpm-buf.c +++ b/drivers/char/tpm/tpm-buf.c @@ -104,13 +104,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length); */ void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length) { - /* Return silently if overflow has already happened. */ - if (buf->flags & TPM_BUF_OVERFLOW) + if (buf->flags & TPM_BUF_INVALID) return; if ((buf->length + new_length) > PAGE_SIZE) { WARN(1, "tpm_buf: write overflow\n"); - buf->flags |= TPM_BUF_OVERFLOW; + buf->flags |= TPM_BUF_INVALID; return; } @@ -157,8 +156,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_u32); */ void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle) { + if (buf->flags & TPM_BUF_INVALID) + return; + if (buf->flags & TPM_BUF_TPM2B) { dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n"); + buf->flags |= TPM_BUF_INVALID; return; } @@ -177,14 +180,13 @@ static void tpm_buf_read(struct tpm_buf *buf, off_t *offset, size_t count, void { off_t next_offset; - /* Return silently if overflow has already happened. */ - if (buf->flags & TPM_BUF_BOUNDARY_ERROR) + if (buf->flags & TPM_BUF_INVALID) return; next_offset = *offset + count; if (next_offset > buf->length) { WARN(1, "tpm_buf: read out of boundary\n"); - buf->flags |= TPM_BUF_BOUNDARY_ERROR; + buf->flags |= TPM_BUF_INVALID; return; } diff --git a/include/linux/tpm.h b/include/linux/tpm.h index e72e7657faa2..5283f32781c4 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -366,12 +366,10 @@ struct tpm_header { } __packed; enum tpm_buf_flags { - /* the capacity exceeded: */ - TPM_BUF_OVERFLOW = BIT(0), /* TPM2B format: */ - TPM_BUF_TPM2B = BIT(1), - /* read out of boundary: */ - TPM_BUF_BOUNDARY_ERROR = BIT(2), + TPM_BUF_TPM2B = BIT(0), + /* The buffer is in invalid and unusable state: */ + TPM_BUF_INVALID = BIT(1), }; /* diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index 8e3b283a59b2..119d5152c0db 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -295,7 +295,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip, /* creation PCR */ tpm_buf_append_u32(&buf, 0); - if (buf.flags & TPM_BUF_OVERFLOW) { + if (buf.flags & TPM_BUF_INVALID) { rc = -E2BIG; tpm2_end_auth_session(chip); goto out; @@ -308,7 +308,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip, goto out; blob_len = tpm_buf_read_u32(&buf, &offset); - if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_BOUNDARY_ERROR) { + if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_INVALID) { rc = -E2BIG; goto out; } @@ -414,7 +414,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, tpm_buf_append(&buf, blob, blob_len); - if (buf.flags & TPM_BUF_OVERFLOW) { + if (buf.flags & TPM_BUF_INVALID) { rc = -E2BIG; tpm2_end_auth_session(chip); goto out; -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/10] tpm-buf: check for corruption in tpm_buf_append_handle() 2025-09-29 19:48 ` [PATCH v3 07/10] tpm-buf: check for corruption in tpm_buf_append_handle() Jarkko Sakkinen @ 2025-09-30 11:13 ` Jonathan McDowell 2025-09-30 12:43 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Jonathan McDowell @ 2025-09-30 11:13 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Mon, Sep 29, 2025 at 10:48:29PM +0300, Jarkko Sakkinen wrote: > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > Unify TPM_BUF_BOUNDARY_ERROR and TPM_BUF_OVERFLOW into TPM_BUF_INVALID > flag because semantically they are identical. > > Test and set TPM_BUF_INVALID in tpm_buf_append_handle() following the > pattern from other functions in tpm-buf.c. > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Reviewed-by: Jonathan McDowell <noodles@meta.com> > --- > v3: > - No changes. > v2: > - A new patch. > --- > drivers/char/tpm/tpm-buf.c | 14 ++++++++------ > include/linux/tpm.h | 8 +++----- > security/keys/trusted-keys/trusted_tpm2.c | 6 +++--- > 3 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c > index dc882fc9fa9e..69ee77400539 100644 > --- a/drivers/char/tpm/tpm-buf.c > +++ b/drivers/char/tpm/tpm-buf.c > @@ -104,13 +104,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length); > */ > void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length) > { > - /* Return silently if overflow has already happened. */ > - if (buf->flags & TPM_BUF_OVERFLOW) > + if (buf->flags & TPM_BUF_INVALID) > return; > > if ((buf->length + new_length) > PAGE_SIZE) { > WARN(1, "tpm_buf: write overflow\n"); > - buf->flags |= TPM_BUF_OVERFLOW; > + buf->flags |= TPM_BUF_INVALID; > return; > } > > @@ -157,8 +156,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_u32); > */ > void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle) > { > + if (buf->flags & TPM_BUF_INVALID) > + return; > + > if (buf->flags & TPM_BUF_TPM2B) { > dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n"); > + buf->flags |= TPM_BUF_INVALID; > return; > } > > @@ -177,14 +180,13 @@ static void tpm_buf_read(struct tpm_buf *buf, off_t *offset, size_t count, void > { > off_t next_offset; > > - /* Return silently if overflow has already happened. */ > - if (buf->flags & TPM_BUF_BOUNDARY_ERROR) > + if (buf->flags & TPM_BUF_INVALID) > return; > > next_offset = *offset + count; > if (next_offset > buf->length) { > WARN(1, "tpm_buf: read out of boundary\n"); > - buf->flags |= TPM_BUF_BOUNDARY_ERROR; > + buf->flags |= TPM_BUF_INVALID; > return; > } > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index e72e7657faa2..5283f32781c4 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -366,12 +366,10 @@ struct tpm_header { > } __packed; > > enum tpm_buf_flags { > - /* the capacity exceeded: */ > - TPM_BUF_OVERFLOW = BIT(0), > /* TPM2B format: */ > - TPM_BUF_TPM2B = BIT(1), > - /* read out of boundary: */ > - TPM_BUF_BOUNDARY_ERROR = BIT(2), > + TPM_BUF_TPM2B = BIT(0), > + /* The buffer is in invalid and unusable state: */ > + TPM_BUF_INVALID = BIT(1), > }; > > /* > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > index 8e3b283a59b2..119d5152c0db 100644 > --- a/security/keys/trusted-keys/trusted_tpm2.c > +++ b/security/keys/trusted-keys/trusted_tpm2.c > @@ -295,7 +295,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > /* creation PCR */ > tpm_buf_append_u32(&buf, 0); > > - if (buf.flags & TPM_BUF_OVERFLOW) { > + if (buf.flags & TPM_BUF_INVALID) { > rc = -E2BIG; > tpm2_end_auth_session(chip); > goto out; > @@ -308,7 +308,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > goto out; > > blob_len = tpm_buf_read_u32(&buf, &offset); > - if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_BOUNDARY_ERROR) { > + if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_INVALID) { > rc = -E2BIG; > goto out; > } > @@ -414,7 +414,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, > > tpm_buf_append(&buf, blob, blob_len); > > - if (buf.flags & TPM_BUF_OVERFLOW) { > + if (buf.flags & TPM_BUF_INVALID) { > rc = -E2BIG; > tpm2_end_auth_session(chip); > goto out; > -- > 2.39.5 > > J. -- Web [ Reality is for people with no grasp of fantasy. ] site: https:// [ ] Made by www.earth.li/~noodles/ [ ] HuggieTag 0.0.24 ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 07/10] tpm-buf: check for corruption in tpm_buf_append_handle() 2025-09-30 11:13 ` Jonathan McDowell @ 2025-09-30 12:43 ` Jarkko Sakkinen 0 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-30 12:43 UTC (permalink / raw) To: Jonathan McDowell Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 12:13:11PM +0100, Jonathan McDowell wrote: > On Mon, Sep 29, 2025 at 10:48:29PM +0300, Jarkko Sakkinen wrote: > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > > > Unify TPM_BUF_BOUNDARY_ERROR and TPM_BUF_OVERFLOW into TPM_BUF_INVALID > > flag because semantically they are identical. > > > > Test and set TPM_BUF_INVALID in tpm_buf_append_handle() following the > > pattern from other functions in tpm-buf.c. > > > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > Reviewed-by: Jonathan McDowell <noodles@meta.com> Thanks. > > > --- > > v3: > > - No changes. > > v2: > > - A new patch. > > --- > > drivers/char/tpm/tpm-buf.c | 14 ++++++++------ > > include/linux/tpm.h | 8 +++----- > > security/keys/trusted-keys/trusted_tpm2.c | 6 +++--- > > 3 files changed, 14 insertions(+), 14 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c > > index dc882fc9fa9e..69ee77400539 100644 > > --- a/drivers/char/tpm/tpm-buf.c > > +++ b/drivers/char/tpm/tpm-buf.c > > @@ -104,13 +104,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length); > > */ > > void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length) > > { > > - /* Return silently if overflow has already happened. */ > > - if (buf->flags & TPM_BUF_OVERFLOW) > > + if (buf->flags & TPM_BUF_INVALID) > > return; > > > > if ((buf->length + new_length) > PAGE_SIZE) { > > WARN(1, "tpm_buf: write overflow\n"); > > - buf->flags |= TPM_BUF_OVERFLOW; > > + buf->flags |= TPM_BUF_INVALID; > > return; > > } > > > > @@ -157,8 +156,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_u32); > > */ > > void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle) > > { > > + if (buf->flags & TPM_BUF_INVALID) > > + return; > > + > > if (buf->flags & TPM_BUF_TPM2B) { > > dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n"); > > + buf->flags |= TPM_BUF_INVALID; > > return; > > } > > > > @@ -177,14 +180,13 @@ static void tpm_buf_read(struct tpm_buf *buf, off_t *offset, size_t count, void > > { > > off_t next_offset; > > > > - /* Return silently if overflow has already happened. */ > > - if (buf->flags & TPM_BUF_BOUNDARY_ERROR) > > + if (buf->flags & TPM_BUF_INVALID) > > return; > > > > next_offset = *offset + count; > > if (next_offset > buf->length) { > > WARN(1, "tpm_buf: read out of boundary\n"); > > - buf->flags |= TPM_BUF_BOUNDARY_ERROR; > > + buf->flags |= TPM_BUF_INVALID; > > return; > > } > > > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > > index e72e7657faa2..5283f32781c4 100644 > > --- a/include/linux/tpm.h > > +++ b/include/linux/tpm.h > > @@ -366,12 +366,10 @@ struct tpm_header { > > } __packed; > > > > enum tpm_buf_flags { > > - /* the capacity exceeded: */ > > - TPM_BUF_OVERFLOW = BIT(0), > > /* TPM2B format: */ > > - TPM_BUF_TPM2B = BIT(1), > > - /* read out of boundary: */ > > - TPM_BUF_BOUNDARY_ERROR = BIT(2), > > + TPM_BUF_TPM2B = BIT(0), > > + /* The buffer is in invalid and unusable state: */ > > + TPM_BUF_INVALID = BIT(1), > > }; > > > > /* > > diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c > > index 8e3b283a59b2..119d5152c0db 100644 > > --- a/security/keys/trusted-keys/trusted_tpm2.c > > +++ b/security/keys/trusted-keys/trusted_tpm2.c > > @@ -295,7 +295,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > > /* creation PCR */ > > tpm_buf_append_u32(&buf, 0); > > > > - if (buf.flags & TPM_BUF_OVERFLOW) { > > + if (buf.flags & TPM_BUF_INVALID) { > > rc = -E2BIG; > > tpm2_end_auth_session(chip); > > goto out; > > @@ -308,7 +308,7 @@ int tpm2_seal_trusted(struct tpm_chip *chip, > > goto out; > > > > blob_len = tpm_buf_read_u32(&buf, &offset); > > - if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_BOUNDARY_ERROR) { > > + if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_INVALID) { > > rc = -E2BIG; > > goto out; > > } > > @@ -414,7 +414,7 @@ static int tpm2_load_cmd(struct tpm_chip *chip, > > > > tpm_buf_append(&buf, blob, blob_len); > > > > - if (buf.flags & TPM_BUF_OVERFLOW) { > > + if (buf.flags & TPM_BUF_INVALID) { > > rc = -E2BIG; > > tpm2_end_auth_session(chip); > > goto out; > > -- > > 2.39.5 > > > > > > J. > > -- > Web [ Reality is for people with no grasp of fantasy. ] > site: https:// [ ] Made by > www.earth.li/~noodles/ [ ] HuggieTag 0.0.24 BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 08/10] tpm-buf: Remove chip parameter from tpm_buf_append_handle 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (6 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 07/10] tpm-buf: check for corruption in tpm_buf_append_handle() Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 11:14 ` Jonathan McDowell 2025-09-29 19:48 ` [PATCH v3 09/10] tpm-buf: Build PCR extend commands Jarkko Sakkinen ` (2 subsequent siblings) 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Remove chip parameter from tpm_buf_append_handle() in order to maintain decoupled state with tpm-buf. This is mandatory change in order to re-use the module in early boot code of Trenchboot, and the binding itself brings no benefit. Use WARN like in other functions, as the error condition can happen only as a net effect of a trivial programming mistake. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - No changes. v2: - A new patch. --- drivers/char/tpm/tpm-buf.c | 5 ++--- drivers/char/tpm/tpm2-cmd.c | 2 +- drivers/char/tpm/tpm2-sessions.c | 2 +- include/linux/tpm.h | 2 +- 4 files changed, 5 insertions(+), 6 deletions(-) diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c index 69ee77400539..1b9dee0d0681 100644 --- a/drivers/char/tpm/tpm-buf.c +++ b/drivers/char/tpm/tpm-buf.c @@ -147,20 +147,19 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_u32); /** * tpm_buf_append_handle() - Add a handle - * @chip: &tpm_chip instance * @buf: &tpm_buf instance * @handle: a TPM object handle * * Add a handle to the buffer, and increase the count tracking the number of * handles in the command buffer. Works only for command buffers. */ -void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle) +void tpm_buf_append_handle(struct tpm_buf *buf, u32 handle) { if (buf->flags & TPM_BUF_INVALID) return; if (buf->flags & TPM_BUF_TPM2B) { - dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n"); + WARN(1, "tpm-buf: invalid type: TPM2B\n"); buf->flags |= TPM_BUF_INVALID; return; } diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index c7bfa705ea8f..b69ff7266450 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -190,7 +190,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, tpm_buf_append_name(chip, &buf, pcr_idx, NULL); tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0); } else { - tpm_buf_append_handle(chip, &buf, pcr_idx); + tpm_buf_append_handle(&buf, pcr_idx); tpm_buf_append_auth(chip, &buf, NULL, 0); } diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c index 13f019d1312a..bbc05f0997a8 100644 --- a/drivers/char/tpm/tpm2-sessions.c +++ b/drivers/char/tpm/tpm2-sessions.c @@ -232,7 +232,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf, #endif if (!tpm2_chip_auth(chip)) { - tpm_buf_append_handle(chip, buf, handle); + tpm_buf_append_handle(buf, handle); return; } diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 5283f32781c4..b2d89df70c18 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -423,7 +423,7 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value); u8 tpm_buf_read_u8(struct tpm_buf *buf, off_t *offset); u16 tpm_buf_read_u16(struct tpm_buf *buf, off_t *offset); u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset); -void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle); +void tpm_buf_append_handle(struct tpm_buf *buf, u32 handle); /* * Check if TPM device is in the firmware upgrade mode. -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 08/10] tpm-buf: Remove chip parameter from tpm_buf_append_handle 2025-09-29 19:48 ` [PATCH v3 08/10] tpm-buf: Remove chip parameter from tpm_buf_append_handle Jarkko Sakkinen @ 2025-09-30 11:14 ` Jonathan McDowell 0 siblings, 0 replies; 31+ messages in thread From: Jonathan McDowell @ 2025-09-30 11:14 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Mon, Sep 29, 2025 at 10:48:30PM +0300, Jarkko Sakkinen wrote: > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > Remove chip parameter from tpm_buf_append_handle() in order to maintain > decoupled state with tpm-buf. This is mandatory change in order to re-use > the module in early boot code of Trenchboot, and the binding itself brings > no benefit. Use WARN like in other functions, as the error condition can > happen only as a net effect of a trivial programming mistake. > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Reviewed-by: Jonathan McDowell <noodles@meta.com> > --- > v3: > - No changes. > v2: > - A new patch. > --- > drivers/char/tpm/tpm-buf.c | 5 ++--- > drivers/char/tpm/tpm2-cmd.c | 2 +- > drivers/char/tpm/tpm2-sessions.c | 2 +- > include/linux/tpm.h | 2 +- > 4 files changed, 5 insertions(+), 6 deletions(-) > > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c > index 69ee77400539..1b9dee0d0681 100644 > --- a/drivers/char/tpm/tpm-buf.c > +++ b/drivers/char/tpm/tpm-buf.c > @@ -147,20 +147,19 @@ EXPORT_SYMBOL_GPL(tpm_buf_append_u32); > > /** > * tpm_buf_append_handle() - Add a handle > - * @chip: &tpm_chip instance > * @buf: &tpm_buf instance > * @handle: a TPM object handle > * > * Add a handle to the buffer, and increase the count tracking the number of > * handles in the command buffer. Works only for command buffers. > */ > -void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle) > +void tpm_buf_append_handle(struct tpm_buf *buf, u32 handle) > { > if (buf->flags & TPM_BUF_INVALID) > return; > > if (buf->flags & TPM_BUF_TPM2B) { > - dev_err(&chip->dev, "Invalid buffer type (TPM2B)\n"); > + WARN(1, "tpm-buf: invalid type: TPM2B\n"); > buf->flags |= TPM_BUF_INVALID; > return; > } > diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c > index c7bfa705ea8f..b69ff7266450 100644 > --- a/drivers/char/tpm/tpm2-cmd.c > +++ b/drivers/char/tpm/tpm2-cmd.c > @@ -190,7 +190,7 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, > tpm_buf_append_name(chip, &buf, pcr_idx, NULL); > tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0); > } else { > - tpm_buf_append_handle(chip, &buf, pcr_idx); > + tpm_buf_append_handle(&buf, pcr_idx); > tpm_buf_append_auth(chip, &buf, NULL, 0); > } > > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c > index 13f019d1312a..bbc05f0997a8 100644 > --- a/drivers/char/tpm/tpm2-sessions.c > +++ b/drivers/char/tpm/tpm2-sessions.c > @@ -232,7 +232,7 @@ void tpm_buf_append_name(struct tpm_chip *chip, struct tpm_buf *buf, > #endif > > if (!tpm2_chip_auth(chip)) { > - tpm_buf_append_handle(chip, buf, handle); > + tpm_buf_append_handle(buf, handle); > return; > } > > diff --git a/include/linux/tpm.h b/include/linux/tpm.h > index 5283f32781c4..b2d89df70c18 100644 > --- a/include/linux/tpm.h > +++ b/include/linux/tpm.h > @@ -423,7 +423,7 @@ void tpm_buf_append_u32(struct tpm_buf *buf, const u32 value); > u8 tpm_buf_read_u8(struct tpm_buf *buf, off_t *offset); > u16 tpm_buf_read_u16(struct tpm_buf *buf, off_t *offset); > u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset); > -void tpm_buf_append_handle(struct tpm_chip *chip, struct tpm_buf *buf, u32 handle); > +void tpm_buf_append_handle(struct tpm_buf *buf, u32 handle); > > /* > * Check if TPM device is in the firmware upgrade mode. > -- > 2.39.5 > > J. -- 101 things you can't have too much of : 24 - Taglines. This .sig brought to you by the letter X and the number 45 Product of the Republic of HuggieTag ^ permalink raw reply [flat|nested] 31+ messages in thread
* [PATCH v3 09/10] tpm-buf: Build PCR extend commands 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (7 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 08/10] tpm-buf: Remove chip parameter from tpm_buf_append_handle Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations Jarkko Sakkinen 2025-09-29 20:10 ` [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen 10 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Build and append TPM_ORD_EXTEND and TPM2_CC_PCR_EXTEND command bodies with the two new functions: 1. tpm1_buf_append_extend() 2. tpm2_buf_append_pcr_extend() These changes make the fallback more informative of the situation, as the underlying programming error is catched at the call site, instead of masking it as a tpm_transmit() failure. Further, decoupling the build of the command bodies for extending PCRs will be mandatory for the Trenchboot early boot code. Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - No changes. v2: - A new patch. --- drivers/char/tpm/tpm-buf.c | 67 +++++++++++++++++++++++++++++++++++++ drivers/char/tpm/tpm1-cmd.c | 15 +++++---- drivers/char/tpm/tpm2-cmd.c | 13 ++++--- include/linux/tpm.h | 4 +++ include/linux/tpm_command.h | 5 +-- 5 files changed, 88 insertions(+), 16 deletions(-) diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c index 1b9dee0d0681..c9e6e5d097ca 100644 --- a/drivers/char/tpm/tpm-buf.c +++ b/drivers/char/tpm/tpm-buf.c @@ -244,4 +244,71 @@ u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset) } EXPORT_SYMBOL_GPL(tpm_buf_read_u32); +static bool tpm1_buf_is_command(struct tpm_buf *buf, u32 ordinal) +{ + struct tpm_header *head = (struct tpm_header *)buf->data; + + return !(buf->flags & TPM_BUF_TPM2B) && + be16_to_cpu(head->tag) == TPM_TAG_RQU_COMMAND && + be32_to_cpu(head->ordinal) == ordinal; +} + +/** + * tpm1_buf_append_extend() - Append command body for TPM_Extend + * @buf: &tpm_buf instance + * @pcr_idx: index of the PCR + * @hash: SHA1 hash + */ +void tpm1_buf_append_extend(struct tpm_buf *buf, u32 pcr_idx, const u8 *hash) +{ + if (buf->flags & TPM_BUF_INVALID) + return; + + if (!tpm1_buf_is_command(buf, TPM_ORD_EXTEND)) { + WARN(1, "tpm_buf: invalid TPM_Extend command\n"); + buf->flags |= TPM_BUF_INVALID; + return; + } + + tpm_buf_append_u32(buf, pcr_idx); + tpm_buf_append(buf, hash, TPM_DIGEST_SIZE); +} + +static bool tpm2_buf_is_command(struct tpm_buf *buf, u32 ordinal) +{ + struct tpm_header *head = (struct tpm_header *)buf->data; + u16 tag = be16_to_cpu(head->tag); + + return !(buf->flags & TPM_BUF_TPM2B) && + (tag == TPM2_ST_SESSIONS || tag == TPM2_ST_NO_SESSIONS) && + be32_to_cpu(head->ordinal) == ordinal; +} + +/** + * tpm2_buf_append_pcr_extend() - Append command body for TPM2_PCR_Extend + * @buf: &tpm_buf instance + * @digests: list of PCR digests + * @banks: PCR bank descriptors + * @nr_banks: number of PCR banks + */ +void tpm2_buf_append_pcr_extend(struct tpm_buf *buf, struct tpm_digest *digests, + struct tpm_bank_info *banks, + unsigned int nr_banks) +{ + int i; + if (buf->flags & TPM_BUF_INVALID) + return; + + if (!tpm2_buf_is_command(buf, TPM2_CC_PCR_EXTEND)) { + WARN(1, "tpm_buf: invalid TPM2_PCR_Extend command\n"); + buf->flags |= TPM_BUF_INVALID; + return; + } + + tpm_buf_append_u32(buf, nr_banks); + for (i = 0; i < nr_banks; i++) { + tpm_buf_append_u16(buf, digests[i].alg_id); + tpm_buf_append(buf, digests[i].digest, banks[i].digest_size); + } +} diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c index 5c49bdff33de..4f1af8beeed4 100644 --- a/drivers/char/tpm/tpm1-cmd.c +++ b/drivers/char/tpm/tpm1-cmd.c @@ -18,8 +18,8 @@ #include <linux/mutex.h> #include <linux/spinlock.h> #include <linux/freezer.h> +#include <linux/tpm_command.h> #include <linux/tpm_eventlog.h> - #include "tpm.h" #define TPM_MAX_ORDINAL 243 @@ -459,21 +459,23 @@ int tpm1_get_timeouts(struct tpm_chip *chip) return 0; } -#define TPM_ORD_PCR_EXTEND 20 int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash, const char *log_msg) { struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCR_EXTEND); + rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_EXTEND); if (rc) return rc; - tpm_buf_append_u32(&buf, pcr_idx); - tpm_buf_append(&buf, hash, TPM_DIGEST_SIZE); + tpm1_buf_append_extend(&buf, pcr_idx, hash); + + if (buf.flags & TPM_BUF_INVALID) + rc = -EINVAL; + else + rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, log_msg); - rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, log_msg); tpm_buf_destroy(&buf); return rc; } @@ -511,7 +513,6 @@ ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap, } EXPORT_SYMBOL_GPL(tpm1_getcap); -#define TPM_ORD_GET_RANDOM 70 struct tpm1_get_random_out { __be32 rng_data_len; u8 rng_data[TPM_MAX_RNG_DATA]; diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index b69ff7266450..d5a22fba32a8 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -171,7 +171,6 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, { struct tpm_buf buf; int rc; - int i; if (!disable_pcr_integrity) { rc = tpm2_start_auth_session(chip); @@ -194,12 +193,12 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, tpm_buf_append_auth(chip, &buf, NULL, 0); } - tpm_buf_append_u32(&buf, chip->nr_allocated_banks); + tpm2_buf_append_pcr_extend(&buf, digests, chip->allocated_banks, + chip->nr_allocated_banks); - for (i = 0; i < chip->nr_allocated_banks; i++) { - tpm_buf_append_u16(&buf, digests[i].alg_id); - tpm_buf_append(&buf, (const unsigned char *)&digests[i].digest, - chip->allocated_banks[i].digest_size); + if (buf.flags & TPM_BUF_INVALID) { + rc = -EINVAL; + goto out; } if (!disable_pcr_integrity) @@ -208,8 +207,8 @@ int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, if (!disable_pcr_integrity) rc = tpm_buf_check_hmac_response(chip, &buf, rc); +out: tpm_buf_destroy(&buf); - return rc; } diff --git a/include/linux/tpm.h b/include/linux/tpm.h index b2d89df70c18..6c7349dce871 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -424,6 +424,10 @@ u8 tpm_buf_read_u8(struct tpm_buf *buf, off_t *offset); u16 tpm_buf_read_u16(struct tpm_buf *buf, off_t *offset); u32 tpm_buf_read_u32(struct tpm_buf *buf, off_t *offset); void tpm_buf_append_handle(struct tpm_buf *buf, u32 handle); +void tpm1_buf_append_extend(struct tpm_buf *buf, u32 pcr_idx, const u8 *hash); +void tpm2_buf_append_pcr_extend(struct tpm_buf *buf, struct tpm_digest *digests, + struct tpm_bank_info *banks, + unsigned int nr_banks); /* * Check if TPM device is in the firmware upgrade mode. diff --git a/include/linux/tpm_command.h b/include/linux/tpm_command.h index f5c03e9c3913..02038972a05f 100644 --- a/include/linux/tpm_command.h +++ b/include/linux/tpm_command.h @@ -16,11 +16,12 @@ #define TPM_TAG_RSP_AUTH2_COMMAND 198 /* Command Ordinals */ -#define TPM_ORD_GETRANDOM 70 -#define TPM_ORD_OSAP 11 #define TPM_ORD_OIAP 10 +#define TPM_ORD_OSAP 11 +#define TPM_ORD_EXTEND 20 #define TPM_ORD_SEAL 23 #define TPM_ORD_UNSEAL 24 +#define TPM_ORD_GET_RANDOM 70 /* Other constants */ #define SRKHANDLE 0x40000000 -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations. 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (8 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 09/10] tpm-buf: Build PCR extend commands Jarkko Sakkinen @ 2025-09-29 19:48 ` Jarkko Sakkinen 2025-09-30 12:44 ` Stefano Garzarella 2025-09-29 20:10 ` [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen 10 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 19:48 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, Jarkko Sakkinen, Stefan Berger, Peter Huewe, Jarkko Sakkinen, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> Decouple kzalloc from buffer creation, so that a managed allocation can be done trivially: struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE, GFP_KERNEL); if (!buf) return -ENOMEM; tpm_buf_init(buf, TPM_BUFSIZE); Alternatively, stack allocations are also possible: u8 buf_data[512]; struct tpm_buf *buf = (struct tpm_buf *)buf_data; tpm_buf_init(buf, sizeof(buf_data)); Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> --- v3: - A new patch from the earlier series with more scoped changes and less abstract commit message. --- drivers/char/tpm/tpm-buf.c | 122 +++++---- drivers/char/tpm/tpm-sysfs.c | 20 +- drivers/char/tpm/tpm.h | 1 - drivers/char/tpm/tpm1-cmd.c | 147 +++++------ drivers/char/tpm/tpm2-cmd.c | 290 ++++++++++------------ drivers/char/tpm/tpm2-sessions.c | 121 +++++---- drivers/char/tpm/tpm2-space.c | 44 ++-- drivers/char/tpm/tpm_vtpm_proxy.c | 30 +-- include/linux/tpm.h | 18 +- security/keys/trusted-keys/trusted_tpm1.c | 34 ++- security/keys/trusted-keys/trusted_tpm2.c | 176 ++++++------- 11 files changed, 482 insertions(+), 521 deletions(-) diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c index c9e6e5d097ca..1cb649938c01 100644 --- a/drivers/char/tpm/tpm-buf.c +++ b/drivers/char/tpm/tpm-buf.c @@ -7,82 +7,109 @@ #include <linux/module.h> #include <linux/tpm.h> -/** - * tpm_buf_init() - Allocate and initialize a TPM command - * @buf: A &tpm_buf - * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS - * @ordinal: A command ordinal - * - * Return: 0 or -ENOMEM - */ -int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal) +static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size) { - buf->data = (u8 *)__get_free_page(GFP_KERNEL); - if (!buf->data) - return -ENOMEM; - - tpm_buf_reset(buf, tag, ordinal); - return 0; + u32 buf_size_2 = (u32)buf->capacity + (u32)sizeof(*buf); + + if (!buf->capacity) { + if (buf_size > TPM_BUFSIZE) { + WARN(1, "%s: size overflow: %u\n", __func__, buf_size); + buf->flags |= TPM_BUF_INVALID; + } + } else { + if (buf_size != buf_size_2) { + WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size, + buf_size_2); + buf->flags |= TPM_BUF_INVALID; + } + } } -EXPORT_SYMBOL_GPL(tpm_buf_init); -/** - * tpm_buf_reset() - Initialize a TPM command - * @buf: A &tpm_buf - * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS - * @ordinal: A command ordinal - */ -void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) +static void __tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal) { struct tpm_header *head = (struct tpm_header *)buf->data; + __tpm_buf_size_invariant(buf, buf_size); + + if (buf->flags & TPM_BUF_INVALID) + return; + WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS && tag != TPM2_ST_SESSIONS && tag != 0); buf->flags = 0; buf->length = sizeof(*head); + buf->capacity = buf_size - sizeof(*buf); + buf->handles = 0; head->tag = cpu_to_be16(tag); head->length = cpu_to_be32(sizeof(*head)); head->ordinal = cpu_to_be32(ordinal); +} + +static void __tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size) +{ + __tpm_buf_size_invariant(buf, buf_size); + + if (buf->flags & TPM_BUF_INVALID) + return; + + buf->flags = TPM_BUF_TPM2B; + buf->length = 2; + buf->capacity = buf_size - sizeof(*buf); buf->handles = 0; + buf->data[0] = 0; + buf->data[1] = 0; } -EXPORT_SYMBOL_GPL(tpm_buf_reset); /** - * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer - * @buf: A @tpm_buf - * - * Return: 0 or -ENOMEM + * tpm_buf_init() - Initialize a TPM command + * @buf: A &tpm_buf + * @buf_size: Size of the buffer. */ -int tpm_buf_init_sized(struct tpm_buf *buf) +void tpm_buf_init(struct tpm_buf *buf, u16 buf_size) { - buf->data = (u8 *)__get_free_page(GFP_KERNEL); - if (!buf->data) - return -ENOMEM; + memset(buf, 0, buf_size); + __tpm_buf_reset(buf, buf_size, TPM_TAG_RQU_COMMAND, 0); +} +EXPORT_SYMBOL_GPL(tpm_buf_init); - tpm_buf_reset_sized(buf); - return 0; +/** + * tpm_buf_init_sized() - Initialize a sized buffer + * @buf: A &tpm_buf + * @buf_size: Size of the buffer. + */ +void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size) +{ + memset(buf, 0, buf_size); + __tpm_buf_reset_sized(buf, buf_size); } EXPORT_SYMBOL_GPL(tpm_buf_init_sized); /** - * tpm_buf_reset_sized() - Initialize a sized buffer + * tpm_buf_reset() - Re-initialize a TPM command * @buf: A &tpm_buf + * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS + * @ordinal: A command ordinal */ -void tpm_buf_reset_sized(struct tpm_buf *buf) +void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) { - buf->flags = TPM_BUF_TPM2B; - buf->length = 2; - buf->data[0] = 0; - buf->data[1] = 0; + u16 buf_size = buf->capacity + sizeof(*buf); + + __tpm_buf_reset(buf, buf_size, tag, ordinal); } -EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); +EXPORT_SYMBOL_GPL(tpm_buf_reset); -void tpm_buf_destroy(struct tpm_buf *buf) +/** + * tpm_buf_reset_sized() - Re-initialize a sized buffer + * @buf: A &tpm_buf + */ +void tpm_buf_reset_sized(struct tpm_buf *buf) { - free_page((unsigned long)buf->data); + u16 buf_size = buf->capacity + sizeof(*buf); + + __tpm_buf_reset_sized(buf, buf_size); } -EXPORT_SYMBOL_GPL(tpm_buf_destroy); +EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); /** * tpm_buf_length() - Return the number of bytes consumed by the data @@ -92,6 +119,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy); */ u32 tpm_buf_length(struct tpm_buf *buf) { + if (buf->flags & TPM_BUF_INVALID) + return 0; + return buf->length; } EXPORT_SYMBOL_GPL(tpm_buf_length); @@ -104,10 +134,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length); */ void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length) { + u32 total_length = (u32)buf->length + (u32)new_length; + if (buf->flags & TPM_BUF_INVALID) return; - if ((buf->length + new_length) > PAGE_SIZE) { + if (total_length > (u32)buf->capacity) { WARN(1, "tpm_buf: write overflow\n"); buf->flags |= TPM_BUF_INVALID; return; diff --git a/drivers/char/tpm/tpm-sysfs.c b/drivers/char/tpm/tpm-sysfs.c index 94231f052ea7..530037a3f4ba 100644 --- a/drivers/char/tpm/tpm-sysfs.c +++ b/drivers/char/tpm/tpm-sysfs.c @@ -32,28 +32,30 @@ struct tpm_readpubek_out { static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, char *buf) { - struct tpm_buf tpm_buf; struct tpm_readpubek_out *out; int i; char *str = buf; struct tpm_chip *chip = to_tpm_chip(dev); char anti_replay[20]; + struct tpm_buf *tpm_buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!tpm_buf) + return -ENOMEM; + memset(&anti_replay, 0, sizeof(anti_replay)); if (tpm_try_get_ops(chip)) return 0; - if (tpm_buf_init(&tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK)) - goto out_ops; - - tpm_buf_append(&tpm_buf, anti_replay, sizeof(anti_replay)); + tpm_buf_init(tpm_buf, TPM_BUFSIZE); + tpm_buf_reset(tpm_buf, TPM_TAG_RQU_COMMAND, TPM_ORD_READPUBEK); + tpm_buf_append(tpm_buf, anti_replay, sizeof(anti_replay)); - if (tpm_transmit_cmd(chip, &tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, + if (tpm_transmit_cmd(chip, tpm_buf, READ_PUBEK_RESULT_MIN_BODY_SIZE, "attempting to read the PUBEK")) - goto out_buf; + goto out_ops; - out = (struct tpm_readpubek_out *)&tpm_buf.data[10]; + out = (struct tpm_readpubek_out *)&tpm_buf->data[10]; str += sprintf(str, "Algorithm: %4ph\n" @@ -71,8 +73,6 @@ static ssize_t pubek_show(struct device *dev, struct device_attribute *attr, for (i = 0; i < 256; i += 16) str += sprintf(str, "%16ph\n", &out->modulus[i]); -out_buf: - tpm_buf_destroy(&tpm_buf); out_ops: tpm_put_ops(chip); return str - buf; diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h index 769fa6b00c54..eaff2055b541 100644 --- a/drivers/char/tpm/tpm.h +++ b/drivers/char/tpm/tpm.h @@ -32,7 +32,6 @@ #endif #define TPM_MINOR 224 /* officially assigned */ -#define TPM_BUFSIZE 4096 #define TPM_NUM_DEVICES 65536 #define TPM_RETRY 50 diff --git a/drivers/char/tpm/tpm1-cmd.c b/drivers/char/tpm/tpm1-cmd.c index 4f1af8beeed4..bc156d7d59f2 100644 --- a/drivers/char/tpm/tpm1-cmd.c +++ b/drivers/char/tpm/tpm1-cmd.c @@ -323,19 +323,19 @@ unsigned long tpm1_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal) */ static int tpm1_startup(struct tpm_chip *chip) { - struct tpm_buf buf; int rc; - dev_info(&chip->dev, "starting up the TPM manually\n"); + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP); - if (rc < 0) - return rc; + dev_info(&chip->dev, "starting up the TPM manually\n"); - tpm_buf_append_u16(&buf, TPM_ST_CLEAR); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_STARTUP); + tpm_buf_append_u16(buf, TPM_ST_CLEAR); - rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM"); - tpm_buf_destroy(&buf); + rc = tpm_transmit_cmd(chip, buf, 0, "attempting to start the TPM"); return rc; } @@ -462,21 +462,19 @@ int tpm1_get_timeouts(struct tpm_chip *chip) int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash, const char *log_msg) { - struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_EXTEND); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - tpm1_buf_append_extend(&buf, pcr_idx, hash); + tpm1_buf_append_extend(buf, pcr_idx, hash); - if (buf.flags & TPM_BUF_INVALID) + if (buf->flags & TPM_BUF_INVALID) rc = -EINVAL; else - rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, log_msg); + rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE, log_msg); - tpm_buf_destroy(&buf); return rc; } @@ -484,31 +482,32 @@ int tpm1_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, const u8 *hash, ssize_t tpm1_getcap(struct tpm_chip *chip, u32 subcap_id, cap_t *cap, const char *desc, size_t min_cap_length) { - struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_CAP); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_CAP); if (subcap_id == TPM_CAP_VERSION_1_1 || subcap_id == TPM_CAP_VERSION_1_2) { - tpm_buf_append_u32(&buf, subcap_id); - tpm_buf_append_u32(&buf, 0); + tpm_buf_append_u32(buf, subcap_id); + tpm_buf_append_u32(buf, 0); } else { if (subcap_id == TPM_CAP_FLAG_PERM || subcap_id == TPM_CAP_FLAG_VOL) - tpm_buf_append_u32(&buf, TPM_CAP_FLAG); + tpm_buf_append_u32(buf, TPM_CAP_FLAG); else - tpm_buf_append_u32(&buf, TPM_CAP_PROP); + tpm_buf_append_u32(buf, TPM_CAP_PROP); - tpm_buf_append_u32(&buf, 4); - tpm_buf_append_u32(&buf, subcap_id); + tpm_buf_append_u32(buf, 4); + tpm_buf_append_u32(buf, subcap_id); } - rc = tpm_transmit_cmd(chip, &buf, min_cap_length, desc); + rc = tpm_transmit_cmd(chip, buf, min_cap_length, desc); if (!rc) - *cap = *(cap_t *)&buf.data[TPM_HEADER_SIZE + 4]; - tpm_buf_destroy(&buf); + *cap = *(cap_t *)&buf->data[TPM_HEADER_SIZE + 4]; return rc; } EXPORT_SYMBOL_GPL(tpm1_getcap); @@ -532,81 +531,71 @@ int tpm1_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { struct tpm1_get_random_out *out; u32 num_bytes = min_t(u32, max, TPM_MAX_RNG_DATA); - struct tpm_buf buf; u32 total = 0; int retries = 5; u32 recd; int rc; - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + tpm_buf_init(buf, TPM_BUFSIZE); do { - tpm_buf_append_u32(&buf, num_bytes); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM); + tpm_buf_append_u32(buf, num_bytes); - rc = tpm_transmit_cmd(chip, &buf, sizeof(out->rng_data_len), + rc = tpm_transmit_cmd(chip, buf, sizeof(out->rng_data_len), "attempting get random"); if (rc) { if (rc > 0) rc = -EIO; - goto out; + return rc; } - out = (struct tpm1_get_random_out *)&buf.data[TPM_HEADER_SIZE]; + out = (struct tpm1_get_random_out *)&buf->data[TPM_HEADER_SIZE]; recd = be32_to_cpu(out->rng_data_len); - if (recd > num_bytes) { - rc = -EFAULT; - goto out; - } + if (recd > num_bytes) + return -EFAULT; + + if (buf->length < TPM_HEADER_SIZE + + sizeof(out->rng_data_len) + recd) + return -EFAULT; - if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + - sizeof(out->rng_data_len) + recd) { - rc = -EFAULT; - goto out; - } memcpy(dest, out->rng_data, recd); dest += recd; total += recd; num_bytes -= recd; - - tpm_buf_reset(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_GET_RANDOM); } while (retries-- && total < max); rc = total ? (int)total : -EIO; -out: - tpm_buf_destroy(&buf); return rc; } #define TPM_ORD_PCRREAD 21 int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf) { - struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - tpm_buf_append_u32(&buf, pcr_idx); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_PCRREAD); + tpm_buf_append_u32(buf, pcr_idx); - rc = tpm_transmit_cmd(chip, &buf, TPM_DIGEST_SIZE, + rc = tpm_transmit_cmd(chip, buf, TPM_DIGEST_SIZE, "attempting to read a pcr value"); if (rc) - goto out; - - if (tpm_buf_length(&buf) < TPM_DIGEST_SIZE) { - rc = -EFAULT; - goto out; - } + return rc; - memcpy(res_buf, &buf.data[TPM_HEADER_SIZE], TPM_DIGEST_SIZE); + if (buf->length < TPM_DIGEST_SIZE) + return -EFAULT; -out: - tpm_buf_destroy(&buf); + memcpy(res_buf, &buf->data[TPM_HEADER_SIZE], TPM_DIGEST_SIZE); return rc; } @@ -620,15 +609,15 @@ int tpm1_pcr_read(struct tpm_chip *chip, u32 pcr_idx, u8 *res_buf) */ static int tpm1_continue_selftest(struct tpm_chip *chip) { - struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_CONTINUE_SELFTEST); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - rc = tpm_transmit_cmd(chip, &buf, 0, "continue selftest"); - tpm_buf_destroy(&buf); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_CONTINUE_SELFTEST); + rc = tpm_transmit_cmd(chip, buf, 0, "continue selftest"); return rc; } @@ -743,22 +732,24 @@ int tpm1_auto_startup(struct tpm_chip *chip) int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr) { u8 dummy_hash[TPM_DIGEST_SIZE] = { 0 }; - struct tpm_buf buf; unsigned int try; int rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; /* for buggy tpm, flush pcrs with extend to selected dummy */ if (tpm_suspend_pcr) rc = tpm1_pcr_extend(chip, tpm_suspend_pcr, dummy_hash, "extending dummy pcr before suspend"); - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE); - if (rc) - return rc; + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE); + /* now do the actual savestate */ for (try = 0; try < TPM_RETRY; try++) { - rc = tpm_transmit_cmd(chip, &buf, 0, NULL); + rc = tpm_transmit_cmd(chip, buf, 0, NULL); /* * If the TPM indicates that it is too busy to respond to * this command then retry before giving up. It can take @@ -773,7 +764,7 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr) break; tpm_msleep(TPM_TIMEOUT_RETRY); - tpm_buf_reset(&buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SAVESTATE); } if (rc) @@ -783,7 +774,5 @@ int tpm1_pm_suspend(struct tpm_chip *chip, u32 tpm_suspend_pcr) dev_warn(&chip->dev, "TPM savestate took %dms\n", try * TPM_TIMEOUT_RETRY); - tpm_buf_destroy(&buf); - return rc; } diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c index d5a22fba32a8..a0fcd3cd00b7 100644 --- a/drivers/char/tpm/tpm2-cmd.c +++ b/drivers/char/tpm/tpm2-cmd.c @@ -104,12 +104,15 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, { int i; int rc; - struct tpm_buf buf; struct tpm2_pcr_read_out *out; u8 pcr_select[TPM2_PCR_SELECT_MIN] = {0}; u16 digest_size; u16 expected_digest_size = 0; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + if (pcr_idx >= TPM2_PLATFORM_PCR) return -EINVAL; @@ -124,36 +127,31 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, expected_digest_size = chip->allocated_banks[i].digest_size; } - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ); - if (rc) - return rc; + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_PCR_READ); pcr_select[pcr_idx >> 3] = 1 << (pcr_idx & 0x7); - tpm_buf_append_u32(&buf, 1); - tpm_buf_append_u16(&buf, digest->alg_id); - tpm_buf_append_u8(&buf, TPM2_PCR_SELECT_MIN); - tpm_buf_append(&buf, (const unsigned char *)pcr_select, + tpm_buf_append_u32(buf, 1); + tpm_buf_append_u16(buf, digest->alg_id); + tpm_buf_append_u8(buf, TPM2_PCR_SELECT_MIN); + tpm_buf_append(buf, (const unsigned char *)pcr_select, sizeof(pcr_select)); - rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to read a pcr value"); + rc = tpm_transmit_cmd(chip, buf, 0, "attempting to read a pcr value"); if (rc) - goto out; + return rc; - out = (struct tpm2_pcr_read_out *)&buf.data[TPM_HEADER_SIZE]; + out = (struct tpm2_pcr_read_out *)&buf->data[TPM_HEADER_SIZE]; digest_size = be16_to_cpu(out->digest_size); if (digest_size > sizeof(digest->digest) || - (!digest_size_ptr && digest_size != expected_digest_size)) { - rc = -EINVAL; - goto out; - } + (!digest_size_ptr && digest_size != expected_digest_size)) + return rc; if (digest_size_ptr) *digest_size_ptr = digest_size; memcpy(digest->digest, out->digest, digest_size); -out: - tpm_buf_destroy(&buf); return rc; } @@ -169,46 +167,41 @@ int tpm2_pcr_read(struct tpm_chip *chip, u32 pcr_idx, int tpm2_pcr_extend(struct tpm_chip *chip, u32 pcr_idx, struct tpm_digest *digests) { - struct tpm_buf buf; int rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + if (!disable_pcr_integrity) { rc = tpm2_start_auth_session(chip); if (rc) return rc; } - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND); - if (rc) { - if (!disable_pcr_integrity) - tpm2_end_auth_session(chip); - return rc; - } + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_PCR_EXTEND); if (!disable_pcr_integrity) { - tpm_buf_append_name(chip, &buf, pcr_idx, NULL); - tpm_buf_append_hmac_session(chip, &buf, 0, NULL, 0); + tpm_buf_append_name(chip, buf, pcr_idx, NULL); + tpm_buf_append_hmac_session(chip, buf, 0, NULL, 0); } else { - tpm_buf_append_handle(&buf, pcr_idx); - tpm_buf_append_auth(chip, &buf, NULL, 0); + tpm_buf_append_handle(buf, pcr_idx); + tpm_buf_append_auth(chip, buf, NULL, 0); } - tpm2_buf_append_pcr_extend(&buf, digests, chip->allocated_banks, + tpm2_buf_append_pcr_extend(buf, digests, chip->allocated_banks, chip->nr_allocated_banks); - if (buf.flags & TPM_BUF_INVALID) { - rc = -EINVAL; - goto out; - } + if (buf->flags & TPM_BUF_INVALID) + return -EINVAL; if (!disable_pcr_integrity) - tpm_buf_fill_hmac_session(chip, &buf); - rc = tpm_transmit_cmd(chip, &buf, 0, "attempting extend a PCR value"); + tpm_buf_fill_hmac_session(chip, buf); + rc = tpm_transmit_cmd(chip, buf, 0, "attempting extend a PCR value"); if (!disable_pcr_integrity) - rc = tpm_buf_check_hmac_response(chip, &buf, rc); + rc = tpm_buf_check_hmac_response(chip, buf, rc); -out: - tpm_buf_destroy(&buf); return rc; } @@ -232,7 +225,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) { struct tpm2_get_random_out *out; struct tpm_header *head; - struct tpm_buf buf; u32 recd; u32 num_bytes = max; int err; @@ -244,51 +236,50 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) if (!num_bytes || max > TPM_MAX_RNG_DATA) return -EINVAL; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + err = tpm2_start_auth_session(chip); if (err) return err; - err = tpm_buf_init(&buf, 0, 0); - if (err) { - tpm2_end_auth_session(chip); - return err; - } - + tpm_buf_init(buf, TPM_BUFSIZE); do { - tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM); + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM); if (tpm2_chip_auth(chip)) { - tpm_buf_append_hmac_session(chip, &buf, + tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT | TPM2_SA_CONTINUE_SESSION, NULL, 0); } else { - offset = buf.handles * 4 + TPM_HEADER_SIZE; - head = (struct tpm_header *)buf.data; - if (tpm_buf_length(&buf) == offset) + offset = buf->handles * 4 + TPM_HEADER_SIZE; + head = (struct tpm_header *)buf->data; + if (tpm_buf_length(buf) == offset) head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); } - tpm_buf_append_u16(&buf, num_bytes); - tpm_buf_fill_hmac_session(chip, &buf); - err = tpm_transmit_cmd(chip, &buf, + tpm_buf_append_u16(buf, num_bytes); + tpm_buf_fill_hmac_session(chip, buf); + err = tpm_transmit_cmd(chip, buf, offsetof(struct tpm2_get_random_out, buffer), "attempting get random"); - err = tpm_buf_check_hmac_response(chip, &buf, err); + err = tpm_buf_check_hmac_response(chip, buf, err); if (err) { if (err > 0) err = -EIO; goto out; } - head = (struct tpm_header *)buf.data; + head = (struct tpm_header *)buf->data; offset = TPM_HEADER_SIZE; /* Skip the parameter size field: */ if (be16_to_cpu(head->tag) == TPM2_ST_SESSIONS) offset += 4; - out = (struct tpm2_get_random_out *)&buf.data[offset]; + out = (struct tpm2_get_random_out *)&buf->data[offset]; recd = min_t(u32, be16_to_cpu(out->size), num_bytes); - if (tpm_buf_length(&buf) < + if (tpm_buf_length(buf) < TPM_HEADER_SIZE + offsetof(struct tpm2_get_random_out, buffer) + recd) { @@ -302,11 +293,9 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) num_bytes -= recd; } while (retries-- && total < max); - tpm_buf_destroy(&buf); - return total ? total : -EIO; + out: - tpm_buf_destroy(&buf); tpm2_end_auth_session(chip); return err; } @@ -318,20 +307,18 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max) */ void tpm2_flush_context(struct tpm_chip *chip, u32 handle) { - struct tpm_buf buf; - int rc; - - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT); - if (rc) { + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) { dev_warn(&chip->dev, "0x%08x was not flushed, out of memory\n", handle); return; } - tpm_buf_append_u32(&buf, handle); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_FLUSH_CONTEXT); + tpm_buf_append_u32(buf, handle); - tpm_transmit_cmd(chip, &buf, 0, "flushing context"); - tpm_buf_destroy(&buf); + tpm_transmit_cmd(chip, buf, 0, "flushing context"); } EXPORT_SYMBOL_GPL(tpm2_flush_context); @@ -358,19 +345,20 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value, const char *desc) { struct tpm2_get_cap_out *out; - struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); - if (rc) - return rc; - tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES); - tpm_buf_append_u32(&buf, property_id); - tpm_buf_append_u32(&buf, 1); - rc = tpm_transmit_cmd(chip, &buf, 0, NULL); + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); + tpm_buf_append_u32(buf, TPM2_CAP_TPM_PROPERTIES); + tpm_buf_append_u32(buf, property_id); + tpm_buf_append_u32(buf, 1); + rc = tpm_transmit_cmd(chip, buf, 0, NULL); if (!rc) { - out = (struct tpm2_get_cap_out *) - &buf.data[TPM_HEADER_SIZE]; + out = (struct tpm2_get_cap_out *)&buf->data[TPM_HEADER_SIZE]; /* * To prevent failing boot up of some systems, Infineon TPM2.0 * returns SUCCESS on TPM2_Startup in field upgrade mode. Also @@ -382,7 +370,6 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id, u32 *value, else rc = -ENODATA; } - tpm_buf_destroy(&buf); return rc; } EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt); @@ -399,15 +386,14 @@ EXPORT_SYMBOL_GPL(tpm2_get_tpm_pt); */ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type) { - struct tpm_buf buf; - int rc; - - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN); - if (rc) + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) return; - tpm_buf_append_u16(&buf, shutdown_type); - tpm_transmit_cmd(chip, &buf, 0, "stopping the TPM"); - tpm_buf_destroy(&buf); + + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SHUTDOWN); + tpm_buf_append_u16(buf, shutdown_type); + tpm_transmit_cmd(chip, buf, 0, "stopping the TPM"); } /** @@ -425,19 +411,20 @@ void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type) */ static int tpm2_do_selftest(struct tpm_chip *chip) { - struct tpm_buf buf; int full; int rc; - for (full = 0; full < 2; full++) { - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - tpm_buf_append_u8(&buf, full); - rc = tpm_transmit_cmd(chip, &buf, 0, + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST); + for (full = 0; full < 2; full++) { + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_SELF_TEST); + tpm_buf_append_u8(buf, full); + rc = tpm_transmit_cmd(chip, buf, 0, "attempting the self test"); - tpm_buf_destroy(&buf); if (rc == TPM2_RC_TESTING) rc = TPM2_RC_SUCCESS; @@ -463,23 +450,24 @@ static int tpm2_do_selftest(struct tpm_chip *chip) int tpm2_probe(struct tpm_chip *chip) { struct tpm_header *out; - struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); - if (rc) - return rc; - tpm_buf_append_u32(&buf, TPM2_CAP_TPM_PROPERTIES); - tpm_buf_append_u32(&buf, TPM_PT_TOTAL_COMMANDS); - tpm_buf_append_u32(&buf, 1); - rc = tpm_transmit_cmd(chip, &buf, 0, NULL); + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); + tpm_buf_append_u32(buf, TPM2_CAP_TPM_PROPERTIES); + tpm_buf_append_u32(buf, TPM_PT_TOTAL_COMMANDS); + tpm_buf_append_u32(buf, 1); + rc = tpm_transmit_cmd(chip, buf, 0, NULL); /* We ignore TPM return codes on purpose. */ if (rc >= 0) { - out = (struct tpm_header *)buf.data; + out = (struct tpm_header *)buf->data; if (be16_to_cpu(out->tag) == TPM2_ST_NO_SESSIONS) chip->flags |= TPM_CHIP_FLAG_TPM2; } - tpm_buf_destroy(&buf); return 0; } EXPORT_SYMBOL_GPL(tpm2_probe); @@ -519,7 +507,6 @@ struct tpm2_pcr_selection { ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) { struct tpm2_pcr_selection pcr_selection; - struct tpm_buf buf; void *marker; void *end; void *pcr_select_offset; @@ -531,39 +518,37 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) int rc; int i = 0; - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - tpm_buf_append_u32(&buf, TPM2_CAP_PCRS); - tpm_buf_append_u32(&buf, 0); - tpm_buf_append_u32(&buf, 1); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); + tpm_buf_append_u32(buf, TPM2_CAP_PCRS); + tpm_buf_append_u32(buf, 0); + tpm_buf_append_u32(buf, 1); - rc = tpm_transmit_cmd(chip, &buf, 9, "get tpm pcr allocation"); + rc = tpm_transmit_cmd(chip, buf, 9, "get tpm pcr allocation"); if (rc) - goto out; + return rc; - nr_possible_banks = be32_to_cpup( - (__be32 *)&buf.data[TPM_HEADER_SIZE + 5]); + nr_possible_banks = be32_to_cpup((__be32 *)&buf->data[TPM_HEADER_SIZE + 5]); if (nr_possible_banks > TPM2_MAX_BANKS) { pr_err("tpm: unexpected number of banks: %u > %u", nr_possible_banks, TPM2_MAX_BANKS); - rc = -ENOMEM; - goto out; + return -ENOMEM; } - marker = &buf.data[TPM_HEADER_SIZE + 9]; + marker = &buf->data[TPM_HEADER_SIZE + 9]; - rsp_len = be32_to_cpup((__be32 *)&buf.data[2]); - end = &buf.data[rsp_len]; + rsp_len = be32_to_cpup((__be32 *)&buf->data[2]); + end = &buf->data[rsp_len]; for (i = 0; i < nr_possible_banks; i++) { pcr_select_offset = marker + offsetof(struct tpm2_pcr_selection, size_of_select); - if (pcr_select_offset >= end) { - rc = -EFAULT; - break; - } + if (pcr_select_offset >= end) + return -EFAULT; memcpy(&pcr_selection, marker, sizeof(pcr_selection)); hash_alg = be16_to_cpu(pcr_selection.hash_alg); @@ -575,7 +560,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) rc = tpm2_init_bank_info(chip, nr_alloc_banks); if (rc < 0) - break; + return rc; nr_alloc_banks++; } @@ -587,21 +572,21 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip) } chip->nr_allocated_banks = nr_alloc_banks; -out: - tpm_buf_destroy(&buf); - - return rc; + return 0; } int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip) { - struct tpm_buf buf; u32 nr_commands; __be32 *attrs; u32 cc; int i; int rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + rc = tpm2_get_tpm_pt(chip, TPM_PT_TOTAL_COMMANDS, &nr_commands, NULL); if (rc) goto out; @@ -618,30 +603,24 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip) goto out; } - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); - if (rc) - goto out; - - tpm_buf_append_u32(&buf, TPM2_CAP_COMMANDS); - tpm_buf_append_u32(&buf, TPM2_CC_FIRST); - tpm_buf_append_u32(&buf, nr_commands); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_CAPABILITY); + tpm_buf_append_u32(buf, TPM2_CAP_COMMANDS); + tpm_buf_append_u32(buf, TPM2_CC_FIRST); + tpm_buf_append_u32(buf, nr_commands); - rc = tpm_transmit_cmd(chip, &buf, 9 + 4 * nr_commands, NULL); - if (rc) { - tpm_buf_destroy(&buf); + rc = tpm_transmit_cmd(chip, buf, 9 + 4 * nr_commands, NULL); + if (rc) goto out; - } - if (nr_commands != - be32_to_cpup((__be32 *)&buf.data[TPM_HEADER_SIZE + 5])) { + if (nr_commands != be32_to_cpup((__be32 *)&buf->data[TPM_HEADER_SIZE + 5])) { rc = -EFAULT; - tpm_buf_destroy(&buf); goto out; } chip->nr_commands = nr_commands; - attrs = (__be32 *)&buf.data[TPM_HEADER_SIZE + 9]; + attrs = (__be32 *)&buf->data[TPM_HEADER_SIZE + 9]; for (i = 0; i < nr_commands; i++, attrs++) { chip->cc_attrs_tbl[i] = be32_to_cpup(attrs); cc = chip->cc_attrs_tbl[i] & 0xFFFF; @@ -653,8 +632,6 @@ int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip) } } - tpm_buf_destroy(&buf); - out: if (rc > 0) rc = -ENODEV; @@ -675,20 +652,17 @@ EXPORT_SYMBOL_GPL(tpm2_get_cc_attrs_tbl); static int tpm2_startup(struct tpm_chip *chip) { - struct tpm_buf buf; - int rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; dev_info(&chip->dev, "starting up the TPM manually\n"); - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP); - if (rc < 0) - return rc; - - tpm_buf_append_u16(&buf, TPM2_SU_CLEAR); - rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to start the TPM"); - tpm_buf_destroy(&buf); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_STARTUP); + tpm_buf_append_u16(buf, TPM2_SU_CLEAR); - return rc; + return tpm_transmit_cmd(chip, buf, 0, "attempting to start the TPM"); } /** diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c index bbc05f0997a8..626554c974a1 100644 --- a/drivers/char/tpm/tpm2-sessions.c +++ b/drivers/char/tpm/tpm2-sessions.c @@ -182,19 +182,18 @@ static int tpm2_parse_read_public(char *name, struct tpm_buf *buf) static int tpm2_read_public(struct tpm_chip *chip, u32 handle, char *name) { - struct tpm_buf buf; int rc; - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - tpm_buf_append_u32(&buf, handle); - rc = tpm_transmit_cmd(chip, &buf, 0, "read public"); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_READ_PUBLIC); + tpm_buf_append_u32(buf, handle); + rc = tpm_transmit_cmd(chip, buf, 0, "read public"); if (rc == TPM2_RC_SUCCESS) - rc = tpm2_parse_read_public(name, &buf); - - tpm_buf_destroy(&buf); + rc = tpm2_parse_read_public(name, buf); return rc; } @@ -924,7 +923,6 @@ static int tpm2_load_null(struct tpm_chip *chip, u32 *null_key) int tpm2_start_auth_session(struct tpm_chip *chip) { struct tpm2_auth *auth; - struct tpm_buf buf; u32 null_key; int rc; @@ -933,6 +931,10 @@ int tpm2_start_auth_session(struct tpm_chip *chip) return 0; } + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + auth = kzalloc(sizeof(*auth), GFP_KERNEL); if (!auth) return -ENOMEM; @@ -943,41 +945,37 @@ int tpm2_start_auth_session(struct tpm_chip *chip) auth->session = TPM_HEADER_SIZE; - rc = tpm_buf_init(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS); - if (rc) - goto out; - + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_NO_SESSIONS, TPM2_CC_START_AUTH_SESS); /* salt key handle */ - tpm_buf_append_u32(&buf, null_key); + tpm_buf_append_u32(buf, null_key); /* bind key handle */ - tpm_buf_append_u32(&buf, TPM2_RH_NULL); + tpm_buf_append_u32(buf, TPM2_RH_NULL); /* nonce caller */ get_random_bytes(auth->our_nonce, sizeof(auth->our_nonce)); - tpm_buf_append_u16(&buf, sizeof(auth->our_nonce)); - tpm_buf_append(&buf, auth->our_nonce, sizeof(auth->our_nonce)); + tpm_buf_append_u16(buf, sizeof(auth->our_nonce)); + tpm_buf_append(buf, auth->our_nonce, sizeof(auth->our_nonce)); /* append encrypted salt and squirrel away unencrypted in auth */ - tpm_buf_append_salt(&buf, chip, auth); + tpm_buf_append_salt(buf, chip, auth); /* session type (HMAC, audit or policy) */ - tpm_buf_append_u8(&buf, TPM2_SE_HMAC); + tpm_buf_append_u8(buf, TPM2_SE_HMAC); /* symmetric encryption parameters */ /* symmetric algorithm */ - tpm_buf_append_u16(&buf, TPM_ALG_AES); + tpm_buf_append_u16(buf, TPM_ALG_AES); /* bits for symmetric algorithm */ - tpm_buf_append_u16(&buf, AES_KEY_BITS); + tpm_buf_append_u16(buf, AES_KEY_BITS); /* symmetric algorithm mode (must be CFB) */ - tpm_buf_append_u16(&buf, TPM_ALG_CFB); + tpm_buf_append_u16(buf, TPM_ALG_CFB); /* hash algorithm for session */ - tpm_buf_append_u16(&buf, TPM_ALG_SHA256); + tpm_buf_append_u16(buf, TPM_ALG_SHA256); - rc = tpm_ret_to_err(tpm_transmit_cmd(chip, &buf, 0, "StartAuthSession")); + rc = tpm_ret_to_err(tpm_transmit_cmd(chip, buf, 0, "StartAuthSession")); tpm2_flush_context(chip, null_key); if (rc == TPM2_RC_SUCCESS) - rc = tpm2_parse_start_auth_session(auth, &buf); - - tpm_buf_destroy(&buf); + rc = tpm2_parse_start_auth_session(auth, buf); if (rc == TPM2_RC_SUCCESS) { chip->auth = auth; @@ -1199,18 +1197,18 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy, u32 *handle, u8 *name) { int rc; - struct tpm_buf buf; - struct tpm_buf template; - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY); - if (rc) - return rc; + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; - rc = tpm_buf_init_sized(&template); - if (rc) { - tpm_buf_destroy(&buf); - return rc; - } + struct tpm_buf *template __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!template) + return -ENOMEM; + + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE_PRIMARY); + tpm_buf_init_sized(template, TPM_BUFSIZE); /* * create the template. Note: in order for userspace to @@ -1222,75 +1220,72 @@ static int tpm2_create_primary(struct tpm_chip *chip, u32 hierarchy, */ /* key type */ - tpm_buf_append_u16(&template, TPM_ALG_ECC); + tpm_buf_append_u16(template, TPM_ALG_ECC); /* name algorithm */ - tpm_buf_append_u16(&template, TPM_ALG_SHA256); + tpm_buf_append_u16(template, TPM_ALG_SHA256); /* object properties */ - tpm_buf_append_u32(&template, TPM2_OA_NULL_KEY); + tpm_buf_append_u32(template, TPM2_OA_NULL_KEY); /* sauth policy (empty) */ - tpm_buf_append_u16(&template, 0); + tpm_buf_append_u16(template, 0); /* BEGIN parameters: key specific; for ECC*/ /* symmetric algorithm */ - tpm_buf_append_u16(&template, TPM_ALG_AES); + tpm_buf_append_u16(template, TPM_ALG_AES); /* bits for symmetric algorithm */ - tpm_buf_append_u16(&template, AES_KEY_BITS); + tpm_buf_append_u16(template, AES_KEY_BITS); /* algorithm mode (must be CFB) */ - tpm_buf_append_u16(&template, TPM_ALG_CFB); + tpm_buf_append_u16(template, TPM_ALG_CFB); /* scheme (NULL means any scheme) */ - tpm_buf_append_u16(&template, TPM_ALG_NULL); + tpm_buf_append_u16(template, TPM_ALG_NULL); /* ECC Curve ID */ - tpm_buf_append_u16(&template, TPM2_ECC_NIST_P256); + tpm_buf_append_u16(template, TPM2_ECC_NIST_P256); /* KDF Scheme */ - tpm_buf_append_u16(&template, TPM_ALG_NULL); + tpm_buf_append_u16(template, TPM_ALG_NULL); /* unique: key specific; for ECC it is two zero size points */ - tpm_buf_append_u16(&template, 0); - tpm_buf_append_u16(&template, 0); + tpm_buf_append_u16(template, 0); + tpm_buf_append_u16(template, 0); /* END parameters */ /* primary handle */ - tpm_buf_append_u32(&buf, hierarchy); - tpm_buf_append_empty_auth(&buf, TPM2_RS_PW); + tpm_buf_append_u32(buf, hierarchy); + tpm_buf_append_empty_auth(buf, TPM2_RS_PW); /* sensitive create size is 4 for two empty buffers */ - tpm_buf_append_u16(&buf, 4); + tpm_buf_append_u16(buf, 4); /* sensitive create auth data (empty) */ - tpm_buf_append_u16(&buf, 0); + tpm_buf_append_u16(buf, 0); /* sensitive create sensitive data (empty) */ - tpm_buf_append_u16(&buf, 0); + tpm_buf_append_u16(buf, 0); /* the public template */ - tpm_buf_append(&buf, template.data, template.length); - tpm_buf_destroy(&template); + tpm_buf_append(buf, template->data, template->length); /* outside info (empty) */ - tpm_buf_append_u16(&buf, 0); + tpm_buf_append_u16(buf, 0); /* creation PCR (none) */ - tpm_buf_append_u32(&buf, 0); + tpm_buf_append_u32(buf, 0); - rc = tpm_transmit_cmd(chip, &buf, 0, + rc = tpm_transmit_cmd(chip, buf, 0, "attempting to create NULL primary"); if (rc == TPM2_RC_SUCCESS) - rc = tpm2_parse_create_primary(chip, &buf, handle, hierarchy, + rc = tpm2_parse_create_primary(chip, buf, handle, hierarchy, name); - tpm_buf_destroy(&buf); - return rc; } diff --git a/drivers/char/tpm/tpm2-space.c b/drivers/char/tpm/tpm2-space.c index 0ad5e18355e0..a2394b6a4b19 100644 --- a/drivers/char/tpm/tpm2-space.c +++ b/drivers/char/tpm/tpm2-space.c @@ -70,24 +70,25 @@ void tpm2_del_space(struct tpm_chip *chip, struct tpm_space *space) int tpm2_load_context(struct tpm_chip *chip, u8 *buf, unsigned int *offset, u32 *handle) { - struct tpm_buf tbuf; struct tpm2_context *ctx; unsigned int body_size; int rc; - rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD); - if (rc) - return rc; + struct tpm_buf *tbuf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!tbuf) + return -ENOMEM; + + tpm_buf_init(tbuf, TPM_BUFSIZE); + tpm_buf_reset(tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_LOAD); ctx = (struct tpm2_context *)&buf[*offset]; body_size = sizeof(*ctx) + be16_to_cpu(ctx->blob_size); - tpm_buf_append(&tbuf, &buf[*offset], body_size); + tpm_buf_append(tbuf, &buf[*offset], body_size); - rc = tpm_transmit_cmd(chip, &tbuf, 4, NULL); + rc = tpm_transmit_cmd(chip, tbuf, 4, NULL); if (rc < 0) { dev_warn(&chip->dev, "%s: failed with a system error %d\n", __func__, rc); - tpm_buf_destroy(&tbuf); return -EFAULT; } else if (tpm2_rc_value(rc) == TPM2_RC_HANDLE || rc == TPM2_RC_REFERENCE_H0) { @@ -102,64 +103,55 @@ int tpm2_load_context(struct tpm_chip *chip, u8 *buf, * flushed outside the space */ *handle = 0; - tpm_buf_destroy(&tbuf); return -ENOENT; } else if (tpm2_rc_value(rc) == TPM2_RC_INTEGRITY) { - tpm_buf_destroy(&tbuf); return -EINVAL; } else if (rc > 0) { dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n", __func__, rc); - tpm_buf_destroy(&tbuf); return -EFAULT; } - *handle = be32_to_cpup((__be32 *)&tbuf.data[TPM_HEADER_SIZE]); + *handle = be32_to_cpup((__be32 *)&tbuf->data[TPM_HEADER_SIZE]); *offset += body_size; - - tpm_buf_destroy(&tbuf); return 0; } int tpm2_save_context(struct tpm_chip *chip, u32 handle, u8 *buf, unsigned int buf_size, unsigned int *offset) { - struct tpm_buf tbuf; unsigned int body_size; int rc; - rc = tpm_buf_init(&tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE); - if (rc) - return rc; + struct tpm_buf *tbuf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!tbuf) + return -ENOMEM; - tpm_buf_append_u32(&tbuf, handle); + tpm_buf_init(tbuf, TPM_BUFSIZE); + tpm_buf_reset(tbuf, TPM2_ST_NO_SESSIONS, TPM2_CC_CONTEXT_SAVE); + tpm_buf_append_u32(tbuf, handle); - rc = tpm_transmit_cmd(chip, &tbuf, 0, NULL); + rc = tpm_transmit_cmd(chip, tbuf, 0, NULL); if (rc < 0) { dev_warn(&chip->dev, "%s: failed with a system error %d\n", __func__, rc); - tpm_buf_destroy(&tbuf); return -EFAULT; } else if (tpm2_rc_value(rc) == TPM2_RC_REFERENCE_H0) { - tpm_buf_destroy(&tbuf); return -ENOENT; } else if (rc) { dev_warn(&chip->dev, "%s: failed with a TPM error 0x%04X\n", __func__, rc); - tpm_buf_destroy(&tbuf); return -EFAULT; } - body_size = tpm_buf_length(&tbuf) - TPM_HEADER_SIZE; + body_size = tpm_buf_length(tbuf) - TPM_HEADER_SIZE; if ((*offset + body_size) > buf_size) { dev_warn(&chip->dev, "%s: out of backing storage\n", __func__); - tpm_buf_destroy(&tbuf); return -ENOMEM; } - memcpy(&buf[*offset], &tbuf.data[TPM_HEADER_SIZE], body_size); + memcpy(&buf[*offset], &tbuf->data[TPM_HEADER_SIZE], body_size); *offset += body_size; - tpm_buf_destroy(&tbuf); return 0; } diff --git a/drivers/char/tpm/tpm_vtpm_proxy.c b/drivers/char/tpm/tpm_vtpm_proxy.c index 0818bb517805..71f3fabae631 100644 --- a/drivers/char/tpm/tpm_vtpm_proxy.c +++ b/drivers/char/tpm/tpm_vtpm_proxy.c @@ -395,40 +395,36 @@ static bool vtpm_proxy_tpm_req_canceled(struct tpm_chip *chip, u8 status) static int vtpm_proxy_request_locality(struct tpm_chip *chip, int locality) { - struct tpm_buf buf; int rc; const struct tpm_header *header; struct proxy_dev *proxy_dev = dev_get_drvdata(&chip->dev); + struct tpm_buf *buf __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + tpm_buf_init(buf, TPM_BUFSIZE); if (chip->flags & TPM_CHIP_FLAG_TPM2) - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, - TPM2_CC_SET_LOCALITY); + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_SET_LOCALITY); else - rc = tpm_buf_init(&buf, TPM_TAG_RQU_COMMAND, - TPM_ORD_SET_LOCALITY); - if (rc) - return rc; - tpm_buf_append_u8(&buf, locality); + tpm_buf_reset(buf, TPM_TAG_RQU_COMMAND, TPM_ORD_SET_LOCALITY); + + tpm_buf_append_u8(buf, locality); proxy_dev->state |= STATE_DRIVER_COMMAND; - rc = tpm_transmit_cmd(chip, &buf, 0, "attempting to set locality"); + rc = tpm_transmit_cmd(chip, buf, 0, "attempting to set locality"); proxy_dev->state &= ~STATE_DRIVER_COMMAND; - if (rc < 0) { - locality = rc; - goto out; - } + if (rc < 0) + return rc; - header = (const struct tpm_header *)buf.data; + header = (const struct tpm_header *)buf->data; rc = be32_to_cpu(header->return_code); if (rc) locality = -1; -out: - tpm_buf_destroy(&buf); - return locality; } diff --git a/include/linux/tpm.h b/include/linux/tpm.h index 6c7349dce871..52859459590e 100644 --- a/include/linux/tpm.h +++ b/include/linux/tpm.h @@ -26,8 +26,9 @@ #include <crypto/hash_info.h> #include <crypto/aes.h> -#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */ +#define TPM_DIGEST_SIZE 20 /* Max TPM v1.2 PCR size */ #define TPM_HEADER_SIZE 10 +#define TPM_BUFSIZE 4096 #define TPM2_PLATFORM_PCR 24 #define TPM2_PCR_SELECT_MIN 3 @@ -373,13 +374,15 @@ enum tpm_buf_flags { }; /* - * A string buffer type for constructing TPM commands. + * A buffer for constructing and parsing TPM commands, responses and sized + * (TPM2B) buffers. */ struct tpm_buf { - u32 flags; - u32 length; - u8 *data; + u8 flags; u8 handles; + u16 length; + u16 capacity; + u8 data[]; }; enum tpm2_object_attributes { @@ -410,11 +413,10 @@ struct tpm2_hash { unsigned int tpm_id; }; -int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal); +void tpm_buf_init(struct tpm_buf *buf, u16 buf_size); +void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size); void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal); -int tpm_buf_init_sized(struct tpm_buf *buf); void tpm_buf_reset_sized(struct tpm_buf *buf); -void tpm_buf_destroy(struct tpm_buf *buf); u32 tpm_buf_length(struct tpm_buf *buf); void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length); void tpm_buf_append_u8(struct tpm_buf *buf, const u8 value); diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c index 636acb66a4f6..6e6a9fb48e63 100644 --- a/security/keys/trusted-keys/trusted_tpm1.c +++ b/security/keys/trusted-keys/trusted_tpm1.c @@ -310,9 +310,8 @@ static int TSS_checkhmac2(unsigned char *buffer, * For key specific tpm requests, we will generate and send our * own TPM command packets using the drivers send function. */ -static int trusted_tpm_send(unsigned char *cmd, size_t buflen) +static int trusted_tpm_send(void *cmd, size_t buflen) { - struct tpm_buf buf; int rc; if (!chip) @@ -322,15 +321,12 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen) if (rc) return rc; - buf.flags = 0; - buf.length = buflen; - buf.data = cmd; dump_tpm_buf(cmd); - rc = tpm_transmit_cmd(chip, &buf, 4, "sending data"); + rc = tpm_transmit_cmd(chip, cmd, 4, "sending data"); dump_tpm_buf(cmd); + /* Convert TPM error to -EPERM. */ if (rc > 0) - /* TPM error */ rc = -EPERM; tpm_put_ops(chip); @@ -624,23 +620,23 @@ static int tpm_unseal(struct tpm_buf *tb, static int key_seal(struct trusted_key_payload *p, struct trusted_key_options *o) { - struct tpm_buf tb; int ret; - ret = tpm_buf_init(&tb, 0, 0); - if (ret) - return ret; + struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!tb) + return -ENOMEM; + + tpm_buf_init(tb, TPM_BUFSIZE); /* include migratable flag at end of sealed key */ p->key[p->key_len] = p->migratable; - ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth, + ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth, p->key, p->key_len + 1, p->blob, &p->blob_len, o->blobauth, o->pcrinfo, o->pcrinfo_len); if (ret < 0) pr_info("srkseal failed (%d)\n", ret); - tpm_buf_destroy(&tb); return ret; } @@ -650,14 +646,15 @@ static int key_seal(struct trusted_key_payload *p, static int key_unseal(struct trusted_key_payload *p, struct trusted_key_options *o) { - struct tpm_buf tb; int ret; - ret = tpm_buf_init(&tb, 0, 0); - if (ret) - return ret; + struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); + if (!tb) + return -ENOMEM; + + tpm_buf_init(tb, TPM_BUFSIZE); - ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, + ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, o->blobauth, p->key, &p->key_len); if (ret < 0) pr_info("srkunseal failed (%d)\n", ret); @@ -665,7 +662,6 @@ static int key_unseal(struct trusted_key_payload *p, /* pull migratable flag out of sealed key */ p->migratable = p->key[--p->key_len]; - tpm_buf_destroy(&tb); return ret; } diff --git a/security/keys/trusted-keys/trusted_tpm2.c b/security/keys/trusted-keys/trusted_tpm2.c index 119d5152c0db..0a99bd051a25 100644 --- a/security/keys/trusted-keys/trusted_tpm2.c +++ b/security/keys/trusted-keys/trusted_tpm2.c @@ -212,13 +212,20 @@ int tpm2_seal_trusted(struct tpm_chip *chip, struct trusted_key_options *options) { off_t offset = TPM_HEADER_SIZE; - struct tpm_buf buf, sized; int blob_len = 0; u32 hash; u32 flags; int i; int rc; + struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + + struct tpm_buf *sized __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL); + if (!sized) + return -ENOMEM; + for (i = 0; i < ARRAY_SIZE(tpm2_hash_map); i++) { if (options->hash == tpm2_hash_map[i].crypto_id) { hash = tpm2_hash_map[i].tpm_id; @@ -240,91 +247,79 @@ int tpm2_seal_trusted(struct tpm_chip *chip, if (rc) goto out_put; - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE); - if (rc) { - tpm2_end_auth_session(chip); - goto out_put; - } - - rc = tpm_buf_init_sized(&sized); - if (rc) { - tpm_buf_destroy(&buf); - tpm2_end_auth_session(chip); - goto out_put; - } - - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL); - tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_DECRYPT, + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_init_sized(sized, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_CREATE); + tpm_buf_append_name(chip, buf, options->keyhandle, NULL); + tpm_buf_append_hmac_session(chip, buf, TPM2_SA_DECRYPT, options->keyauth, TPM_DIGEST_SIZE); /* sensitive */ - tpm_buf_append_u16(&sized, options->blobauth_len); + tpm_buf_append_u16(sized, options->blobauth_len); if (options->blobauth_len) - tpm_buf_append(&sized, options->blobauth, options->blobauth_len); + tpm_buf_append(sized, options->blobauth, options->blobauth_len); - tpm_buf_append_u16(&sized, payload->key_len); - tpm_buf_append(&sized, payload->key, payload->key_len); - tpm_buf_append(&buf, sized.data, sized.length); + tpm_buf_append_u16(sized, payload->key_len); + tpm_buf_append(sized, payload->key, payload->key_len); + tpm_buf_append(buf, sized->data, sized->length); /* public */ - tpm_buf_reset_sized(&sized); - tpm_buf_append_u16(&sized, TPM_ALG_KEYEDHASH); - tpm_buf_append_u16(&sized, hash); + tpm_buf_init_sized(sized, TPM_BUFSIZE); + tpm_buf_append_u16(sized, TPM_ALG_KEYEDHASH); + tpm_buf_append_u16(sized, hash); /* key properties */ flags = 0; flags |= options->policydigest_len ? 0 : TPM2_OA_USER_WITH_AUTH; flags |= payload->migratable ? 0 : (TPM2_OA_FIXED_TPM | TPM2_OA_FIXED_PARENT); - tpm_buf_append_u32(&sized, flags); + tpm_buf_append_u32(sized, flags); /* policy */ - tpm_buf_append_u16(&sized, options->policydigest_len); + tpm_buf_append_u16(sized, options->policydigest_len); if (options->policydigest_len) - tpm_buf_append(&sized, options->policydigest, options->policydigest_len); + tpm_buf_append(sized, options->policydigest, options->policydigest_len); /* public parameters */ - tpm_buf_append_u16(&sized, TPM_ALG_NULL); - tpm_buf_append_u16(&sized, 0); + tpm_buf_append_u16(sized, TPM_ALG_NULL); + tpm_buf_append_u16(sized, 0); - tpm_buf_append(&buf, sized.data, sized.length); + tpm_buf_append(buf, sized->data, sized->length); /* outside info */ - tpm_buf_append_u16(&buf, 0); + tpm_buf_append_u16(buf, 0); /* creation PCR */ - tpm_buf_append_u32(&buf, 0); + tpm_buf_append_u32(buf, 0); - if (buf.flags & TPM_BUF_INVALID) { + if (buf->flags & TPM_BUF_INVALID) { rc = -E2BIG; tpm2_end_auth_session(chip); goto out; } - tpm_buf_fill_hmac_session(chip, &buf); - rc = tpm_transmit_cmd(chip, &buf, 4, "sealing data"); - rc = tpm_buf_check_hmac_response(chip, &buf, rc); + tpm_buf_fill_hmac_session(chip, buf); + rc = tpm_transmit_cmd(chip, buf, 4, "sealing data"); + rc = tpm_buf_check_hmac_response(chip, buf, rc); if (rc) goto out; - blob_len = tpm_buf_read_u32(&buf, &offset); - if (blob_len > MAX_BLOB_SIZE || buf.flags & TPM_BUF_INVALID) { + blob_len = tpm_buf_read_u32(buf, &offset); + if (blob_len > MAX_BLOB_SIZE || buf->flags & TPM_BUF_INVALID) { rc = -E2BIG; goto out; } - if (buf.length - offset < blob_len) { + if (buf->length - offset < blob_len) { rc = -EFAULT; goto out; } - blob_len = tpm2_key_encode(payload, options, &buf.data[offset], blob_len); - if (blob_len < 0) - rc = blob_len; + blob_len = tpm2_key_encode(payload, options, &buf->data[offset], + blob_len); out: - tpm_buf_destroy(&sized); - tpm_buf_destroy(&buf); - + if (blob_len < 0) + rc = blob_len; if (!rc) payload->blob_len = blob_len; @@ -351,7 +346,6 @@ static int tpm2_load_cmd(struct tpm_chip *chip, struct trusted_key_options *options, u32 *blob_handle) { - struct tpm_buf buf; unsigned int private_len; unsigned int public_len; unsigned int blob_len; @@ -359,6 +353,10 @@ static int tpm2_load_cmd(struct tpm_chip *chip, int rc; u32 attrs; + struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + rc = tpm2_key_decode(payload, options, &blob); if (rc) { /* old form */ @@ -402,35 +400,30 @@ static int tpm2_load_cmd(struct tpm_chip *chip, if (rc) return rc; - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD); - if (rc) { - tpm2_end_auth_session(chip); - return rc; - } - - tpm_buf_append_name(chip, &buf, options->keyhandle, NULL); - tpm_buf_append_hmac_session(chip, &buf, 0, options->keyauth, + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_LOAD); + tpm_buf_append_name(chip, buf, options->keyhandle, NULL); + tpm_buf_append_hmac_session(chip, buf, 0, options->keyauth, TPM_DIGEST_SIZE); - tpm_buf_append(&buf, blob, blob_len); + tpm_buf_append(buf, blob, blob_len); - if (buf.flags & TPM_BUF_INVALID) { + if (buf->flags & TPM_BUF_INVALID) { rc = -E2BIG; tpm2_end_auth_session(chip); goto out; } - tpm_buf_fill_hmac_session(chip, &buf); - rc = tpm_transmit_cmd(chip, &buf, 4, "loading blob"); - rc = tpm_buf_check_hmac_response(chip, &buf, rc); + tpm_buf_fill_hmac_session(chip, buf); + rc = tpm_transmit_cmd(chip, buf, 4, "loading blob"); + rc = tpm_buf_check_hmac_response(chip, buf, rc); if (!rc) *blob_handle = be32_to_cpup( - (__be32 *) &buf.data[TPM_HEADER_SIZE]); + (__be32 *)&buf->data[TPM_HEADER_SIZE]); out: if (blob != payload->blob) kfree(blob); - tpm_buf_destroy(&buf); return tpm_ret_to_err(rc); } @@ -453,26 +446,25 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, u32 blob_handle) { struct tpm_header *head; - struct tpm_buf buf; u16 data_len; int offset; u8 *data; int rc; + struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL); + if (!buf) + return -ENOMEM; + rc = tpm2_start_auth_session(chip); if (rc) return rc; - rc = tpm_buf_init(&buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL); - if (rc) { - tpm2_end_auth_session(chip); - return rc; - } - - tpm_buf_append_name(chip, &buf, blob_handle, NULL); + tpm_buf_init(buf, TPM_BUFSIZE); + tpm_buf_reset(buf, TPM2_ST_SESSIONS, TPM2_CC_UNSEAL); + tpm_buf_append_name(chip, buf, blob_handle, NULL); if (!options->policyhandle) { - tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, + tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT, options->blobauth, options->blobauth_len); } else { @@ -481,39 +473,35 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, * the password will end up being unencrypted on the bus, as * HMAC nonce cannot be calculated for it. */ - tpm_buf_append_u32(&buf, 9 + options->blobauth_len); - tpm_buf_append_u32(&buf, options->policyhandle); - tpm_buf_append_u16(&buf, 0); - tpm_buf_append_u8(&buf, 0); - tpm_buf_append_u16(&buf, options->blobauth_len); - tpm_buf_append(&buf, options->blobauth, options->blobauth_len); + tpm_buf_append_u32(buf, 9 + options->blobauth_len); + tpm_buf_append_u32(buf, options->policyhandle); + tpm_buf_append_u16(buf, 0); + tpm_buf_append_u8(buf, 0); + tpm_buf_append_u16(buf, options->blobauth_len); + tpm_buf_append(buf, options->blobauth, options->blobauth_len); if (tpm2_chip_auth(chip)) { - tpm_buf_append_hmac_session(chip, &buf, TPM2_SA_ENCRYPT, NULL, 0); + tpm_buf_append_hmac_session(chip, buf, TPM2_SA_ENCRYPT, NULL, 0); } else { - offset = buf.handles * 4 + TPM_HEADER_SIZE; - head = (struct tpm_header *)buf.data; - if (tpm_buf_length(&buf) == offset) + offset = buf->handles * 4 + TPM_HEADER_SIZE; + head = (struct tpm_header *)buf->data; + if (tpm_buf_length(buf) == offset) head->tag = cpu_to_be16(TPM2_ST_NO_SESSIONS); } } - tpm_buf_fill_hmac_session(chip, &buf); - rc = tpm_transmit_cmd(chip, &buf, 6, "unsealing"); - rc = tpm_buf_check_hmac_response(chip, &buf, rc); + tpm_buf_fill_hmac_session(chip, buf); + rc = tpm_transmit_cmd(chip, buf, 6, "unsealing"); + rc = tpm_buf_check_hmac_response(chip, buf, rc); if (!rc) { data_len = be16_to_cpup( - (__be16 *) &buf.data[TPM_HEADER_SIZE + 4]); - if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE) { - rc = -EFAULT; - goto out; - } + (__be16 *)&buf->data[TPM_HEADER_SIZE + 4]); + if (data_len < MIN_KEY_SIZE || data_len > MAX_KEY_SIZE) + return -EFAULT; - if (tpm_buf_length(&buf) < TPM_HEADER_SIZE + 6 + data_len) { - rc = -EFAULT; - goto out; - } - data = &buf.data[TPM_HEADER_SIZE + 6]; + if (tpm_buf_length(buf) < TPM_HEADER_SIZE + 6 + data_len) + return -EFAULT; + data = &buf->data[TPM_HEADER_SIZE + 6]; if (payload->old_format) { /* migratable flag is at the end of the key */ @@ -530,8 +518,6 @@ static int tpm2_unseal_cmd(struct tpm_chip *chip, } } -out: - tpm_buf_destroy(&buf); return tpm_ret_to_err(rc); } -- 2.39.5 ^ permalink raw reply related [flat|nested] 31+ messages in thread
* Re: [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations. 2025-09-29 19:48 ` [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations Jarkko Sakkinen @ 2025-09-30 12:44 ` Stefano Garzarella 2025-09-30 13:11 ` Jarkko Sakkinen 0 siblings, 1 reply; 31+ messages in thread From: Stefano Garzarella @ 2025-09-30 12:44 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Stefan Berger, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Mon, Sep 29, 2025 at 10:48:32PM +0300, Jarkko Sakkinen wrote: >From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > >Decouple kzalloc from buffer creation, so that a managed allocation can be >done trivially: > > struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE, ^ In the code, we use PAGE_SIZE instead of TPM_BUFSIZE with kzalloc(). Should we do the same in this example? (Perhaps adding the reason, if you think it would be useful) > GFP_KERNEL); > if (!buf) > return -ENOMEM; > tpm_buf_init(buf, TPM_BUFSIZE); > >Alternatively, stack allocations are also possible: > > u8 buf_data[512]; > struct tpm_buf *buf = (struct tpm_buf *)buf_data; > tpm_buf_init(buf, sizeof(buf_data)); > >Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> >Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> >--- >v3: >- A new patch from the earlier series with more scoped changes and > less abstract commit message. >--- > drivers/char/tpm/tpm-buf.c | 122 +++++---- > drivers/char/tpm/tpm-sysfs.c | 20 +- > drivers/char/tpm/tpm.h | 1 - > drivers/char/tpm/tpm1-cmd.c | 147 +++++------ > drivers/char/tpm/tpm2-cmd.c | 290 ++++++++++------------ > drivers/char/tpm/tpm2-sessions.c | 121 +++++---- > drivers/char/tpm/tpm2-space.c | 44 ++-- > drivers/char/tpm/tpm_vtpm_proxy.c | 30 +-- > include/linux/tpm.h | 18 +- > security/keys/trusted-keys/trusted_tpm1.c | 34 ++- > security/keys/trusted-keys/trusted_tpm2.c | 176 ++++++------- > 11 files changed, 482 insertions(+), 521 deletions(-) > >diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c >index c9e6e5d097ca..1cb649938c01 100644 >--- a/drivers/char/tpm/tpm-buf.c >+++ b/drivers/char/tpm/tpm-buf.c >@@ -7,82 +7,109 @@ > #include <linux/module.h> > #include <linux/tpm.h> > >-/** >- * tpm_buf_init() - Allocate and initialize a TPM command >- * @buf: A &tpm_buf >- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS >- * @ordinal: A command ordinal >- * >- * Return: 0 or -ENOMEM >- */ >-int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal) >+static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size) > { >- buf->data = (u8 *)__get_free_page(GFP_KERNEL); >- if (!buf->data) >- return -ENOMEM; >- >- tpm_buf_reset(buf, tag, ordinal); >- return 0; >+ u32 buf_size_2 = (u32)buf->capacity + (u32)sizeof(*buf); >+ >+ if (!buf->capacity) { >+ if (buf_size > TPM_BUFSIZE) { >+ WARN(1, "%s: size overflow: %u\n", __func__, buf_size); >+ buf->flags |= TPM_BUF_INVALID; >+ } >+ } else { >+ if (buf_size != buf_size_2) { >+ WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size, >+ buf_size_2); >+ buf->flags |= TPM_BUF_INVALID; >+ } >+ } > } >-EXPORT_SYMBOL_GPL(tpm_buf_init); > >-/** >- * tpm_buf_reset() - Initialize a TPM command >- * @buf: A &tpm_buf >- * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS >- * @ordinal: A command ordinal >- */ >-void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) >+static void __tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal) > { > struct tpm_header *head = (struct tpm_header *)buf->data; > >+ __tpm_buf_size_invariant(buf, buf_size); >+ >+ if (buf->flags & TPM_BUF_INVALID) >+ return; >+ > WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS && > tag != TPM2_ST_SESSIONS && tag != 0); > > buf->flags = 0; > buf->length = sizeof(*head); >+ buf->capacity = buf_size - sizeof(*buf); >+ buf->handles = 0; > head->tag = cpu_to_be16(tag); > head->length = cpu_to_be32(sizeof(*head)); > head->ordinal = cpu_to_be32(ordinal); >+} >+ >+static void __tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size) >+{ >+ __tpm_buf_size_invariant(buf, buf_size); >+ >+ if (buf->flags & TPM_BUF_INVALID) >+ return; >+ >+ buf->flags = TPM_BUF_TPM2B; >+ buf->length = 2; >+ buf->capacity = buf_size - sizeof(*buf); > buf->handles = 0; >+ buf->data[0] = 0; >+ buf->data[1] = 0; > } >-EXPORT_SYMBOL_GPL(tpm_buf_reset); > > /** >- * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer >- * @buf: A @tpm_buf >- * >- * Return: 0 or -ENOMEM >+ * tpm_buf_init() - Initialize a TPM command >+ * @buf: A &tpm_buf >+ * @buf_size: Size of the buffer. > */ >-int tpm_buf_init_sized(struct tpm_buf *buf) >+void tpm_buf_init(struct tpm_buf *buf, u16 buf_size) > { >- buf->data = (u8 *)__get_free_page(GFP_KERNEL); >- if (!buf->data) >- return -ENOMEM; >+ memset(buf, 0, buf_size); >+ __tpm_buf_reset(buf, buf_size, TPM_TAG_RQU_COMMAND, 0); >+} >+EXPORT_SYMBOL_GPL(tpm_buf_init); > >- tpm_buf_reset_sized(buf); >- return 0; >+/** >+ * tpm_buf_init_sized() - Initialize a sized buffer >+ * @buf: A &tpm_buf >+ * @buf_size: Size of the buffer. >+ */ >+void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size) >+{ >+ memset(buf, 0, buf_size); >+ __tpm_buf_reset_sized(buf, buf_size); > } > EXPORT_SYMBOL_GPL(tpm_buf_init_sized); > > /** >- * tpm_buf_reset_sized() - Initialize a sized buffer >+ * tpm_buf_reset() - Re-initialize a TPM command > * @buf: A &tpm_buf >+ * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS >+ * @ordinal: A command ordinal > */ >-void tpm_buf_reset_sized(struct tpm_buf *buf) >+void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) > { >- buf->flags = TPM_BUF_TPM2B; >- buf->length = 2; >- buf->data[0] = 0; >- buf->data[1] = 0; >+ u16 buf_size = buf->capacity + sizeof(*buf); >+ >+ __tpm_buf_reset(buf, buf_size, tag, ordinal); > } >-EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); >+EXPORT_SYMBOL_GPL(tpm_buf_reset); > >-void tpm_buf_destroy(struct tpm_buf *buf) >+/** >+ * tpm_buf_reset_sized() - Re-initialize a sized buffer >+ * @buf: A &tpm_buf >+ */ >+void tpm_buf_reset_sized(struct tpm_buf *buf) > { >- free_page((unsigned long)buf->data); >+ u16 buf_size = buf->capacity + sizeof(*buf); >+ >+ __tpm_buf_reset_sized(buf, buf_size); > } >-EXPORT_SYMBOL_GPL(tpm_buf_destroy); >+EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); > > /** > * tpm_buf_length() - Return the number of bytes consumed by the data >@@ -92,6 +119,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy); > */ > u32 tpm_buf_length(struct tpm_buf *buf) Should we update the return value to u16? > { >+ if (buf->flags & TPM_BUF_INVALID) >+ return 0; >+ > return buf->length; > } > EXPORT_SYMBOL_GPL(tpm_buf_length); >@@ -104,10 +134,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length); > */ > void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length) > { >+ u32 total_length = (u32)buf->length + (u32)new_length; >+ > if (buf->flags & TPM_BUF_INVALID) > return; > >- if ((buf->length + new_length) > PAGE_SIZE) { >+ if (total_length > (u32)buf->capacity) { > WARN(1, "tpm_buf: write overflow\n"); > buf->flags |= TPM_BUF_INVALID; > return; [...] >diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c >index 636acb66a4f6..6e6a9fb48e63 100644 >--- a/security/keys/trusted-keys/trusted_tpm1.c >+++ b/security/keys/trusted-keys/trusted_tpm1.c >@@ -310,9 +310,8 @@ static int TSS_checkhmac2(unsigned char *buffer, > * For key specific tpm requests, we will generate and send our > * own TPM command packets using the drivers send function. > */ >-static int trusted_tpm_send(unsigned char *cmd, size_t buflen) >+static int trusted_tpm_send(void *cmd, size_t buflen) > { >- struct tpm_buf buf; > int rc; > > if (!chip) >@@ -322,15 +321,12 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen) > if (rc) > return rc; > >- buf.flags = 0; >- buf.length = buflen; >- buf.data = cmd; > dump_tpm_buf(cmd); >- rc = tpm_transmit_cmd(chip, &buf, 4, "sending data"); >+ rc = tpm_transmit_cmd(chip, cmd, 4, "sending data"); Is it fine here to remove the intermediate tpm_buf ? IIUC tpm_transmit_cmd() needs a tpm_buf, while here we are passing just the "data", or in some way it's a nested tpm_buf? (Sorry if it's a stupid question, but I'm still a bit new with this code). > dump_tpm_buf(cmd); > >+ /* Convert TPM error to -EPERM. */ > if (rc > 0) >- /* TPM error */ > rc = -EPERM; > > tpm_put_ops(chip); >@@ -624,23 +620,23 @@ static int tpm_unseal(struct tpm_buf *tb, > static int key_seal(struct trusted_key_payload *p, > struct trusted_key_options *o) > { >- struct tpm_buf tb; > int ret; > >- ret = tpm_buf_init(&tb, 0, 0); >- if (ret) >- return ret; >+ struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); >+ if (!tb) >+ return -ENOMEM; >+ >+ tpm_buf_init(tb, TPM_BUFSIZE); > > /* include migratable flag at end of sealed key */ > p->key[p->key_len] = p->migratable; > >- ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth, >+ ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth, > p->key, p->key_len + 1, p->blob, &p->blob_len, > o->blobauth, o->pcrinfo, o->pcrinfo_len); > if (ret < 0) > pr_info("srkseal failed (%d)\n", ret); > >- tpm_buf_destroy(&tb); > return ret; > } > >@@ -650,14 +646,15 @@ static int key_seal(struct trusted_key_payload *p, > static int key_unseal(struct trusted_key_payload *p, > struct trusted_key_options *o) > { >- struct tpm_buf tb; > int ret; > >- ret = tpm_buf_init(&tb, 0, 0); >- if (ret) >- return ret; >+ struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, >GFP_KERNEL); >+ if (!tb) >+ return -ENOMEM; >+ >+ tpm_buf_init(tb, TPM_BUFSIZE); > >- ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, >+ ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, > o->blobauth, p->key, &p->key_len); > if (ret < 0) > pr_info("srkunseal failed (%d)\n", ret); >@@ -665,7 +662,6 @@ static int key_unseal(struct trusted_key_payload *p, > /* pull migratable flag out of sealed key */ > p->migratable = p->key[--p->key_len]; > >- tpm_buf_destroy(&tb); > return ret; > } The rest LGTM, but it's a huge patch (not your issue at all), so yeah not sure :-) Thanks, Stefano ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations. 2025-09-30 12:44 ` Stefano Garzarella @ 2025-09-30 13:11 ` Jarkko Sakkinen 2025-09-30 13:20 ` Stefano Garzarella 0 siblings, 1 reply; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-30 13:11 UTC (permalink / raw) To: Stefano Garzarella Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Stefan Berger, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 02:44:52PM +0200, Stefano Garzarella wrote: > On Mon, Sep 29, 2025 at 10:48:32PM +0300, Jarkko Sakkinen wrote: > > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > > > Decouple kzalloc from buffer creation, so that a managed allocation can be > > done trivially: > > > > struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE, > ^ > In the code, we use PAGE_SIZE instead of TPM_BUFSIZE with kzalloc(). > Should we do the same in this example? (Perhaps adding the reason, if you > think it would be useful) I think that should be fixed up in the patch and use TPM_BUFSIZE consistently for kzallocs. Thanks for the remark. > > > GFP_KERNEL); > > if (!buf) > > return -ENOMEM; > > tpm_buf_init(buf, TPM_BUFSIZE); > > > > Alternatively, stack allocations are also possible: > > > > u8 buf_data[512]; > > struct tpm_buf *buf = (struct tpm_buf *)buf_data; > > tpm_buf_init(buf, sizeof(buf_data)); > > > > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> > > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> > > --- > > v3: > > - A new patch from the earlier series with more scoped changes and > > less abstract commit message. > > --- > > drivers/char/tpm/tpm-buf.c | 122 +++++---- > > drivers/char/tpm/tpm-sysfs.c | 20 +- > > drivers/char/tpm/tpm.h | 1 - > > drivers/char/tpm/tpm1-cmd.c | 147 +++++------ > > drivers/char/tpm/tpm2-cmd.c | 290 ++++++++++------------ > > drivers/char/tpm/tpm2-sessions.c | 121 +++++---- > > drivers/char/tpm/tpm2-space.c | 44 ++-- > > drivers/char/tpm/tpm_vtpm_proxy.c | 30 +-- > > include/linux/tpm.h | 18 +- > > security/keys/trusted-keys/trusted_tpm1.c | 34 ++- > > security/keys/trusted-keys/trusted_tpm2.c | 176 ++++++------- > > 11 files changed, 482 insertions(+), 521 deletions(-) > > > > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c > > index c9e6e5d097ca..1cb649938c01 100644 > > --- a/drivers/char/tpm/tpm-buf.c > > +++ b/drivers/char/tpm/tpm-buf.c > > @@ -7,82 +7,109 @@ > > #include <linux/module.h> > > #include <linux/tpm.h> > > > > -/** > > - * tpm_buf_init() - Allocate and initialize a TPM command > > - * @buf: A &tpm_buf > > - * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS > > - * @ordinal: A command ordinal > > - * > > - * Return: 0 or -ENOMEM > > - */ > > -int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal) > > +static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size) > > { > > - buf->data = (u8 *)__get_free_page(GFP_KERNEL); > > - if (!buf->data) > > - return -ENOMEM; > > - > > - tpm_buf_reset(buf, tag, ordinal); > > - return 0; > > + u32 buf_size_2 = (u32)buf->capacity + (u32)sizeof(*buf); > > + > > + if (!buf->capacity) { > > + if (buf_size > TPM_BUFSIZE) { > > + WARN(1, "%s: size overflow: %u\n", __func__, buf_size); > > + buf->flags |= TPM_BUF_INVALID; > > + } > > + } else { > > + if (buf_size != buf_size_2) { > > + WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size, > > + buf_size_2); > > + buf->flags |= TPM_BUF_INVALID; > > + } > > + } > > } > > -EXPORT_SYMBOL_GPL(tpm_buf_init); > > > > -/** > > - * tpm_buf_reset() - Initialize a TPM command > > - * @buf: A &tpm_buf > > - * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS > > - * @ordinal: A command ordinal > > - */ > > -void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) > > +static void __tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal) > > { > > struct tpm_header *head = (struct tpm_header *)buf->data; > > > > + __tpm_buf_size_invariant(buf, buf_size); > > + > > + if (buf->flags & TPM_BUF_INVALID) > > + return; > > + > > WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS && > > tag != TPM2_ST_SESSIONS && tag != 0); > > > > buf->flags = 0; > > buf->length = sizeof(*head); > > + buf->capacity = buf_size - sizeof(*buf); > > + buf->handles = 0; > > head->tag = cpu_to_be16(tag); > > head->length = cpu_to_be32(sizeof(*head)); > > head->ordinal = cpu_to_be32(ordinal); > > +} > > + > > +static void __tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size) > > +{ > > + __tpm_buf_size_invariant(buf, buf_size); > > + > > + if (buf->flags & TPM_BUF_INVALID) > > + return; > > + > > + buf->flags = TPM_BUF_TPM2B; > > + buf->length = 2; > > + buf->capacity = buf_size - sizeof(*buf); > > buf->handles = 0; > > + buf->data[0] = 0; > > + buf->data[1] = 0; > > } > > -EXPORT_SYMBOL_GPL(tpm_buf_reset); > > > > /** > > - * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer > > - * @buf: A @tpm_buf > > - * > > - * Return: 0 or -ENOMEM > > + * tpm_buf_init() - Initialize a TPM command > > + * @buf: A &tpm_buf > > + * @buf_size: Size of the buffer. > > */ > > -int tpm_buf_init_sized(struct tpm_buf *buf) > > +void tpm_buf_init(struct tpm_buf *buf, u16 buf_size) > > { > > - buf->data = (u8 *)__get_free_page(GFP_KERNEL); > > - if (!buf->data) > > - return -ENOMEM; > > + memset(buf, 0, buf_size); > > + __tpm_buf_reset(buf, buf_size, TPM_TAG_RQU_COMMAND, 0); > > +} > > +EXPORT_SYMBOL_GPL(tpm_buf_init); > > > > - tpm_buf_reset_sized(buf); > > - return 0; > > +/** > > + * tpm_buf_init_sized() - Initialize a sized buffer > > + * @buf: A &tpm_buf > > + * @buf_size: Size of the buffer. > > + */ > > +void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size) > > +{ > > + memset(buf, 0, buf_size); > > + __tpm_buf_reset_sized(buf, buf_size); > > } > > EXPORT_SYMBOL_GPL(tpm_buf_init_sized); > > > > /** > > - * tpm_buf_reset_sized() - Initialize a sized buffer > > + * tpm_buf_reset() - Re-initialize a TPM command > > * @buf: A &tpm_buf > > + * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS > > + * @ordinal: A command ordinal > > */ > > -void tpm_buf_reset_sized(struct tpm_buf *buf) > > +void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) > > { > > - buf->flags = TPM_BUF_TPM2B; > > - buf->length = 2; > > - buf->data[0] = 0; > > - buf->data[1] = 0; > > + u16 buf_size = buf->capacity + sizeof(*buf); > > + > > + __tpm_buf_reset(buf, buf_size, tag, ordinal); > > } > > -EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); > > +EXPORT_SYMBOL_GPL(tpm_buf_reset); > > > > -void tpm_buf_destroy(struct tpm_buf *buf) > > +/** > > + * tpm_buf_reset_sized() - Re-initialize a sized buffer > > + * @buf: A &tpm_buf > > + */ > > +void tpm_buf_reset_sized(struct tpm_buf *buf) > > { > > - free_page((unsigned long)buf->data); > > + u16 buf_size = buf->capacity + sizeof(*buf); > > + > > + __tpm_buf_reset_sized(buf, buf_size); > > } > > -EXPORT_SYMBOL_GPL(tpm_buf_destroy); > > +EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); > > > > /** > > * tpm_buf_length() - Return the number of bytes consumed by the data > > @@ -92,6 +119,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy); > > */ > > u32 tpm_buf_length(struct tpm_buf *buf) > > Should we update the return value to u16? > > > { > > + if (buf->flags & TPM_BUF_INVALID) > > + return 0; > > + > > return buf->length; > > } > > EXPORT_SYMBOL_GPL(tpm_buf_length); > > @@ -104,10 +134,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length); > > */ > > void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length) > > { > > + u32 total_length = (u32)buf->length + (u32)new_length; > > + > > if (buf->flags & TPM_BUF_INVALID) > > return; > > > > - if ((buf->length + new_length) > PAGE_SIZE) { > > + if (total_length > (u32)buf->capacity) { > > WARN(1, "tpm_buf: write overflow\n"); > > buf->flags |= TPM_BUF_INVALID; > > return; > > [...] > > > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c > > index 636acb66a4f6..6e6a9fb48e63 100644 > > --- a/security/keys/trusted-keys/trusted_tpm1.c > > +++ b/security/keys/trusted-keys/trusted_tpm1.c > > @@ -310,9 +310,8 @@ static int TSS_checkhmac2(unsigned char *buffer, > > * For key specific tpm requests, we will generate and send our > > * own TPM command packets using the drivers send function. > > */ > > -static int trusted_tpm_send(unsigned char *cmd, size_t buflen) > > +static int trusted_tpm_send(void *cmd, size_t buflen) > > { > > - struct tpm_buf buf; > > int rc; > > > > if (!chip) > > @@ -322,15 +321,12 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen) > > if (rc) > > return rc; > > > > - buf.flags = 0; > > - buf.length = buflen; > > - buf.data = cmd; > > dump_tpm_buf(cmd); > > - rc = tpm_transmit_cmd(chip, &buf, 4, "sending data"); > > + rc = tpm_transmit_cmd(chip, cmd, 4, "sending data"); > > Is it fine here to remove the intermediate tpm_buf ? > > IIUC tpm_transmit_cmd() needs a tpm_buf, while here we are passing just > the "data", or in some way it's a nested tpm_buf? > > (Sorry if it's a stupid question, but I'm still a bit new with this code). > > > dump_tpm_buf(cmd); > > > > + /* Convert TPM error to -EPERM. */ > > if (rc > 0) > > - /* TPM error */ > > rc = -EPERM; > > > > tpm_put_ops(chip); > > @@ -624,23 +620,23 @@ static int tpm_unseal(struct tpm_buf *tb, > > static int key_seal(struct trusted_key_payload *p, > > struct trusted_key_options *o) > > { > > - struct tpm_buf tb; > > int ret; > > > > - ret = tpm_buf_init(&tb, 0, 0); > > - if (ret) > > - return ret; > > + struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); > > + if (!tb) > > + return -ENOMEM; > > + > > + tpm_buf_init(tb, TPM_BUFSIZE); > > > > /* include migratable flag at end of sealed key */ > > p->key[p->key_len] = p->migratable; > > > > - ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth, > > + ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth, > > p->key, p->key_len + 1, p->blob, &p->blob_len, > > o->blobauth, o->pcrinfo, o->pcrinfo_len); > > if (ret < 0) > > pr_info("srkseal failed (%d)\n", ret); > > > > - tpm_buf_destroy(&tb); > > return ret; > > } > > > > @@ -650,14 +646,15 @@ static int key_seal(struct trusted_key_payload *p, > > static int key_unseal(struct trusted_key_payload *p, > > struct trusted_key_options *o) > > { > > - struct tpm_buf tb; > > int ret; > > > > - ret = tpm_buf_init(&tb, 0, 0); > > - if (ret) > > - return ret; > > + struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); > > + if (!tb) > > + return -ENOMEM; > > + > > + tpm_buf_init(tb, TPM_BUFSIZE); > > > > - ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, > > + ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, > > o->blobauth, p->key, &p->key_len); > > if (ret < 0) > > pr_info("srkunseal failed (%d)\n", ret); > > @@ -665,7 +662,6 @@ static int key_unseal(struct trusted_key_payload *p, > > /* pull migratable flag out of sealed key */ > > p->migratable = p->key[--p->key_len]; > > > > - tpm_buf_destroy(&tb); > > return ret; > > } > > The rest LGTM, but it's a huge patch (not your issue at all), so yeah not > sure :-) It's still a single logical change :-) > > Thanks, > Stefano > BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations. 2025-09-30 13:11 ` Jarkko Sakkinen @ 2025-09-30 13:20 ` Stefano Garzarella 0 siblings, 0 replies; 31+ messages in thread From: Stefano Garzarella @ 2025-09-30 13:20 UTC (permalink / raw) To: Jarkko Sakkinen Cc: linux-integrity, dpsmith, ross.philipson, Jarkko Sakkinen, Stefan Berger, Peter Huewe, Jason Gunthorpe, David Howells, Paul Moore, James Morris, Serge E. Hallyn, James Bottomley, Mimi Zohar, open list, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM On Tue, Sep 30, 2025 at 04:11:00PM +0300, Jarkko Sakkinen wrote: >On Tue, Sep 30, 2025 at 02:44:52PM +0200, Stefano Garzarella wrote: >> On Mon, Sep 29, 2025 at 10:48:32PM +0300, Jarkko Sakkinen wrote: >> > From: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> >> > >> > Decouple kzalloc from buffer creation, so that a managed allocation can be >> > done trivially: >> > >> > struct tpm_buf *buf __free(kfree) buf = kzalloc(TPM_BUFSIZE, >> ^ >> In the code, we use PAGE_SIZE instead of TPM_BUFSIZE with kzalloc(). >> Should we do the same in this example? (Perhaps adding the reason, if you >> think it would be useful) > >I think that should be fixed up in the patch and use TPM_BUFSIZE >consistently for kzallocs. Thanks for the remark. I thought it was done on purpose to alleviate pressure on the allocator regardless of the page size value. In any case, it's up to you, I would just be consistent between the example in the commit and the code. > >> >> > GFP_KERNEL); >> > if (!buf) >> > return -ENOMEM; >> > tpm_buf_init(buf, TPM_BUFSIZE); >> > >> > Alternatively, stack allocations are also possible: >> > >> > u8 buf_data[512]; >> > struct tpm_buf *buf = (struct tpm_buf *)buf_data; >> > tpm_buf_init(buf, sizeof(buf_data)); >> > >> > Reviewed-by: Stefan Berger <stefanb@linux.ibm.com> >> > Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@opinsys.com> >> > --- >> > v3: >> > - A new patch from the earlier series with more scoped changes and >> > less abstract commit message. >> > --- >> > drivers/char/tpm/tpm-buf.c | 122 +++++---- >> > drivers/char/tpm/tpm-sysfs.c | 20 +- >> > drivers/char/tpm/tpm.h | 1 - >> > drivers/char/tpm/tpm1-cmd.c | 147 +++++------ >> > drivers/char/tpm/tpm2-cmd.c | 290 ++++++++++------------ >> > drivers/char/tpm/tpm2-sessions.c | 121 +++++---- >> > drivers/char/tpm/tpm2-space.c | 44 ++-- >> > drivers/char/tpm/tpm_vtpm_proxy.c | 30 +-- >> > include/linux/tpm.h | 18 +- >> > security/keys/trusted-keys/trusted_tpm1.c | 34 ++- >> > security/keys/trusted-keys/trusted_tpm2.c | 176 ++++++------- >> > 11 files changed, 482 insertions(+), 521 deletions(-) >> > >> > diff --git a/drivers/char/tpm/tpm-buf.c b/drivers/char/tpm/tpm-buf.c >> > index c9e6e5d097ca..1cb649938c01 100644 >> > --- a/drivers/char/tpm/tpm-buf.c >> > +++ b/drivers/char/tpm/tpm-buf.c >> > @@ -7,82 +7,109 @@ >> > #include <linux/module.h> >> > #include <linux/tpm.h> >> > >> > -/** >> > - * tpm_buf_init() - Allocate and initialize a TPM command >> > - * @buf: A &tpm_buf >> > - * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS >> > - * @ordinal: A command ordinal >> > - * >> > - * Return: 0 or -ENOMEM >> > - */ >> > -int tpm_buf_init(struct tpm_buf *buf, u16 tag, u32 ordinal) >> > +static void __tpm_buf_size_invariant(struct tpm_buf *buf, u16 buf_size) >> > { >> > - buf->data = (u8 *)__get_free_page(GFP_KERNEL); >> > - if (!buf->data) >> > - return -ENOMEM; >> > - >> > - tpm_buf_reset(buf, tag, ordinal); >> > - return 0; >> > + u32 buf_size_2 = (u32)buf->capacity + (u32)sizeof(*buf); >> > + >> > + if (!buf->capacity) { >> > + if (buf_size > TPM_BUFSIZE) { >> > + WARN(1, "%s: size overflow: %u\n", __func__, buf_size); >> > + buf->flags |= TPM_BUF_INVALID; >> > + } >> > + } else { >> > + if (buf_size != buf_size_2) { >> > + WARN(1, "%s: size mismatch: %u != %u\n", __func__, buf_size, >> > + buf_size_2); >> > + buf->flags |= TPM_BUF_INVALID; >> > + } >> > + } >> > } >> > -EXPORT_SYMBOL_GPL(tpm_buf_init); >> > >> > -/** >> > - * tpm_buf_reset() - Initialize a TPM command >> > - * @buf: A &tpm_buf >> > - * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS >> > - * @ordinal: A command ordinal >> > - */ >> > -void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) >> > +static void __tpm_buf_reset(struct tpm_buf *buf, u16 buf_size, u16 tag, u32 ordinal) >> > { >> > struct tpm_header *head = (struct tpm_header *)buf->data; >> > >> > + __tpm_buf_size_invariant(buf, buf_size); >> > + >> > + if (buf->flags & TPM_BUF_INVALID) >> > + return; >> > + >> > WARN_ON(tag != TPM_TAG_RQU_COMMAND && tag != TPM2_ST_NO_SESSIONS && >> > tag != TPM2_ST_SESSIONS && tag != 0); >> > >> > buf->flags = 0; >> > buf->length = sizeof(*head); >> > + buf->capacity = buf_size - sizeof(*buf); >> > + buf->handles = 0; >> > head->tag = cpu_to_be16(tag); >> > head->length = cpu_to_be32(sizeof(*head)); >> > head->ordinal = cpu_to_be32(ordinal); >> > +} >> > + >> > +static void __tpm_buf_reset_sized(struct tpm_buf *buf, u16 buf_size) >> > +{ >> > + __tpm_buf_size_invariant(buf, buf_size); >> > + >> > + if (buf->flags & TPM_BUF_INVALID) >> > + return; >> > + >> > + buf->flags = TPM_BUF_TPM2B; >> > + buf->length = 2; >> > + buf->capacity = buf_size - sizeof(*buf); >> > buf->handles = 0; >> > + buf->data[0] = 0; >> > + buf->data[1] = 0; >> > } >> > -EXPORT_SYMBOL_GPL(tpm_buf_reset); >> > >> > /** >> > - * tpm_buf_init_sized() - Allocate and initialize a sized (TPM2B) buffer >> > - * @buf: A @tpm_buf >> > - * >> > - * Return: 0 or -ENOMEM >> > + * tpm_buf_init() - Initialize a TPM command >> > + * @buf: A &tpm_buf >> > + * @buf_size: Size of the buffer. >> > */ >> > -int tpm_buf_init_sized(struct tpm_buf *buf) >> > +void tpm_buf_init(struct tpm_buf *buf, u16 buf_size) >> > { >> > - buf->data = (u8 *)__get_free_page(GFP_KERNEL); >> > - if (!buf->data) >> > - return -ENOMEM; >> > + memset(buf, 0, buf_size); >> > + __tpm_buf_reset(buf, buf_size, TPM_TAG_RQU_COMMAND, 0); >> > +} >> > +EXPORT_SYMBOL_GPL(tpm_buf_init); >> > >> > - tpm_buf_reset_sized(buf); >> > - return 0; >> > +/** >> > + * tpm_buf_init_sized() - Initialize a sized buffer >> > + * @buf: A &tpm_buf >> > + * @buf_size: Size of the buffer. >> > + */ >> > +void tpm_buf_init_sized(struct tpm_buf *buf, u16 buf_size) >> > +{ >> > + memset(buf, 0, buf_size); >> > + __tpm_buf_reset_sized(buf, buf_size); >> > } >> > EXPORT_SYMBOL_GPL(tpm_buf_init_sized); >> > >> > /** >> > - * tpm_buf_reset_sized() - Initialize a sized buffer >> > + * tpm_buf_reset() - Re-initialize a TPM command >> > * @buf: A &tpm_buf >> > + * @tag: TPM_TAG_RQU_COMMAND, TPM2_ST_NO_SESSIONS or TPM2_ST_SESSIONS >> > + * @ordinal: A command ordinal >> > */ >> > -void tpm_buf_reset_sized(struct tpm_buf *buf) >> > +void tpm_buf_reset(struct tpm_buf *buf, u16 tag, u32 ordinal) >> > { >> > - buf->flags = TPM_BUF_TPM2B; >> > - buf->length = 2; >> > - buf->data[0] = 0; >> > - buf->data[1] = 0; >> > + u16 buf_size = buf->capacity + sizeof(*buf); >> > + >> > + __tpm_buf_reset(buf, buf_size, tag, ordinal); >> > } >> > -EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); >> > +EXPORT_SYMBOL_GPL(tpm_buf_reset); >> > >> > -void tpm_buf_destroy(struct tpm_buf *buf) >> > +/** >> > + * tpm_buf_reset_sized() - Re-initialize a sized buffer >> > + * @buf: A &tpm_buf >> > + */ >> > +void tpm_buf_reset_sized(struct tpm_buf *buf) >> > { >> > - free_page((unsigned long)buf->data); >> > + u16 buf_size = buf->capacity + sizeof(*buf); >> > + >> > + __tpm_buf_reset_sized(buf, buf_size); >> > } >> > -EXPORT_SYMBOL_GPL(tpm_buf_destroy); >> > +EXPORT_SYMBOL_GPL(tpm_buf_reset_sized); >> > >> > /** >> > * tpm_buf_length() - Return the number of bytes consumed by the data >> > @@ -92,6 +119,9 @@ EXPORT_SYMBOL_GPL(tpm_buf_destroy); >> > */ >> > u32 tpm_buf_length(struct tpm_buf *buf) >> >> Should we update the return value to u16? >> >> > { >> > + if (buf->flags & TPM_BUF_INVALID) >> > + return 0; >> > + >> > return buf->length; >> > } >> > EXPORT_SYMBOL_GPL(tpm_buf_length); >> > @@ -104,10 +134,12 @@ EXPORT_SYMBOL_GPL(tpm_buf_length); >> > */ >> > void tpm_buf_append(struct tpm_buf *buf, const u8 *new_data, u16 new_length) >> > { >> > + u32 total_length = (u32)buf->length + (u32)new_length; >> > + >> > if (buf->flags & TPM_BUF_INVALID) >> > return; >> > >> > - if ((buf->length + new_length) > PAGE_SIZE) { >> > + if (total_length > (u32)buf->capacity) { >> > WARN(1, "tpm_buf: write overflow\n"); >> > buf->flags |= TPM_BUF_INVALID; >> > return; >> >> [...] >> >> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c >> > index 636acb66a4f6..6e6a9fb48e63 100644 >> > --- a/security/keys/trusted-keys/trusted_tpm1.c >> > +++ b/security/keys/trusted-keys/trusted_tpm1.c >> > @@ -310,9 +310,8 @@ static int TSS_checkhmac2(unsigned char *buffer, >> > * For key specific tpm requests, we will generate and send our >> > * own TPM command packets using the drivers send function. >> > */ >> > -static int trusted_tpm_send(unsigned char *cmd, size_t buflen) >> > +static int trusted_tpm_send(void *cmd, size_t buflen) >> > { >> > - struct tpm_buf buf; >> > int rc; >> > >> > if (!chip) >> > @@ -322,15 +321,12 @@ static int trusted_tpm_send(unsigned char *cmd, size_t buflen) >> > if (rc) >> > return rc; >> > >> > - buf.flags = 0; >> > - buf.length = buflen; >> > - buf.data = cmd; >> > dump_tpm_buf(cmd); >> > - rc = tpm_transmit_cmd(chip, &buf, 4, "sending data"); >> > + rc = tpm_transmit_cmd(chip, cmd, 4, "sending data"); >> >> Is it fine here to remove the intermediate tpm_buf ? >> >> IIUC tpm_transmit_cmd() needs a tpm_buf, while here we are passing just >> the "data", or in some way it's a nested tpm_buf? >> >> (Sorry if it's a stupid question, but I'm still a bit new with this code). >> >> > dump_tpm_buf(cmd); >> > >> > + /* Convert TPM error to -EPERM. */ >> > if (rc > 0) >> > - /* TPM error */ >> > rc = -EPERM; >> > >> > tpm_put_ops(chip); >> > @@ -624,23 +620,23 @@ static int tpm_unseal(struct tpm_buf *tb, >> > static int key_seal(struct trusted_key_payload *p, >> > struct trusted_key_options *o) >> > { >> > - struct tpm_buf tb; >> > int ret; >> > >> > - ret = tpm_buf_init(&tb, 0, 0); >> > - if (ret) >> > - return ret; >> > + struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); >> > + if (!tb) >> > + return -ENOMEM; >> > + >> > + tpm_buf_init(tb, TPM_BUFSIZE); >> > >> > /* include migratable flag at end of sealed key */ >> > p->key[p->key_len] = p->migratable; >> > >> > - ret = tpm_seal(&tb, o->keytype, o->keyhandle, o->keyauth, >> > + ret = tpm_seal(tb, o->keytype, o->keyhandle, o->keyauth, >> > p->key, p->key_len + 1, p->blob, &p->blob_len, >> > o->blobauth, o->pcrinfo, o->pcrinfo_len); >> > if (ret < 0) >> > pr_info("srkseal failed (%d)\n", ret); >> > >> > - tpm_buf_destroy(&tb); >> > return ret; >> > } >> > >> > @@ -650,14 +646,15 @@ static int key_seal(struct trusted_key_payload *p, >> > static int key_unseal(struct trusted_key_payload *p, >> > struct trusted_key_options *o) >> > { >> > - struct tpm_buf tb; >> > int ret; >> > >> > - ret = tpm_buf_init(&tb, 0, 0); >> > - if (ret) >> > - return ret; >> > + struct tpm_buf *tb __free(kfree) = kzalloc(PAGE_SIZE, GFP_KERNEL); >> > + if (!tb) >> > + return -ENOMEM; >> > + >> > + tpm_buf_init(tb, TPM_BUFSIZE); >> > >> > - ret = tpm_unseal(&tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, >> > + ret = tpm_unseal(tb, o->keyhandle, o->keyauth, p->blob, p->blob_len, >> > o->blobauth, p->key, &p->key_len); >> > if (ret < 0) >> > pr_info("srkunseal failed (%d)\n", ret); >> > @@ -665,7 +662,6 @@ static int key_unseal(struct trusted_key_payload *p, >> > /* pull migratable flag out of sealed key */ >> > p->migratable = p->key[--p->key_len]; >> > >> > - tpm_buf_destroy(&tb); >> > return ret; >> > } >> >> The rest LGTM, but it's a huge patch (not your issue at all), so yeah not >> sure :-) > >It's still a single logical change :-) Yep, and good set of patches to prepare this! BTW I'm still bit concerned about the changes in trusted_tpm_send() which I left a few lines above, can you check it? Thanks, Stefano ^ permalink raw reply [flat|nested] 31+ messages in thread
* Re: [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen ` (9 preceding siblings ...) 2025-09-29 19:48 ` [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations Jarkko Sakkinen @ 2025-09-29 20:10 ` Jarkko Sakkinen 10 siblings, 0 replies; 31+ messages in thread From: Jarkko Sakkinen @ 2025-09-29 20:10 UTC (permalink / raw) To: linux-integrity Cc: dpsmith, ross.philipson, David Howells, Paul Moore, James Morris, Serge E. Hallyn, open list:KEYS/KEYRINGS, open list:SECURITY SUBSYSTEM, open list On Mon, Sep 29, 2025 at 10:48:22PM +0300, Jarkko Sakkinen wrote: > Overview > ======== > > Decouple TPM driver features relevant for Trenchboot and make tpm-buf > robust and decoupled entity from the rest of driver. By doing this, code > can be easily linked to the early boot code. > > Backlog > ======= > > Parts of tpm_tis should separated and decouple from the driver code so that > the slices of code can be compiled into early boot code. Since by other > means the series already has most of the gaps filled it's better to resolve > this issue before landing the series. I.e. goal is that redundancy is mostly in the glue not in replicated code that we want to fix regressions at one place. These patches address end-to-end efficient building and parsing of PCR extends with super-relaxed mm-requirements (stack will do), and tpm_tis is the final piece. BR, Jarkko ^ permalink raw reply [flat|nested] 31+ messages in thread
end of thread, other threads:[~2025-10-01 12:53 UTC | newest] Thread overview: 31+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2025-09-29 19:48 [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 01/10] tpm: Cap the number of PCR banks Jarkko Sakkinen 2025-09-30 11:09 ` Jonathan McDowell 2025-09-30 12:36 ` Jarkko Sakkinen 2025-09-30 14:17 ` James Bottomley 2025-10-01 11:16 ` Jarkko Sakkinen 2025-10-01 12:52 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 02/10] tpm: Use -EPERM as fallback error code in tpm_ret_to_err Jarkko Sakkinen 2025-09-30 12:11 ` Stefano Garzarella 2025-09-30 12:37 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 03/10] KEYS: trusted: Use tpm_ret_to_err() in trusted_tpm2 Jarkko Sakkinen 2025-09-30 12:12 ` Stefano Garzarella 2025-09-30 12:39 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 04/10] tpm2-sessions: Remove 'attributes' from tpm_buf_append_auth Jarkko Sakkinen 2025-09-30 11:10 ` Jonathan McDowell 2025-09-30 12:39 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 05/10] tpm2-sessions: Umask tpm_buf_append_hmac_session() Jarkko Sakkinen 2025-09-30 11:11 ` Jonathan McDowell 2025-09-30 12:41 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 06/10] KEYS: trusted: Open code tpm2_buf_append() Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 07/10] tpm-buf: check for corruption in tpm_buf_append_handle() Jarkko Sakkinen 2025-09-30 11:13 ` Jonathan McDowell 2025-09-30 12:43 ` Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 08/10] tpm-buf: Remove chip parameter from tpm_buf_append_handle Jarkko Sakkinen 2025-09-30 11:14 ` Jonathan McDowell 2025-09-29 19:48 ` [PATCH v3 09/10] tpm-buf: Build PCR extend commands Jarkko Sakkinen 2025-09-29 19:48 ` [PATCH v3 10/10] tpm-buf: Enable managed and stack allocations Jarkko Sakkinen 2025-09-30 12:44 ` Stefano Garzarella 2025-09-30 13:11 ` Jarkko Sakkinen 2025-09-30 13:20 ` Stefano Garzarella 2025-09-29 20:10 ` [PATCH v3 00/10] tpm: Decouple Trenchboot dependencies Jarkko Sakkinen
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).