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.5On 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
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
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
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
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
© 2016 - 2025 Red Hat, Inc.