On Mon, Sep 28, 2026 at 04:27:00PM +0800, Yuqi Xu wrote:
> The TPM2_CAP_PCRS response is external input. tpm2_get_pcr_allocation()
> adds every active selection to allocated_banks, while
> tpm_sysfs_add_device() has one group slot per distinct hash algorithm.
> Eight duplicate selections can therefore write past chip->groups[].
>
> The TPM protocol does not allow a hash algorithm to appear twice in
> this capability response. Treat a duplicate as a broken TPM and return
> -EIO so the chip is not enabled, instead of silently dropping the
> extra selection.
>
> Fixes: aab73d952402 ("tpm: add sysfs exports for all banks of PCR registers")
> Cc: [email protected]
> Reported-by: Vega <[email protected]>
> Assisted-by: LLM
> Signed-off-by: Yuqi Xu <[email protected]>
> Reviewed-by: Ren Wei <[email protected]>
> ---
> Changes in v2:
> - Return -EIO on a duplicate PCR bank instead of silently ignoring
> it, as requested by Jarkko Sakkinen. A duplicate violates the TPM
> protocol, so the chip is not enabled.
> - v1 Link:
> https://lore.kernel.org/all/[email protected]/
> drivers/char/tpm/tpm2-cmd.c | 18 ++++++++++++++++++
> 1 file changed, 18 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index ae22295df798..a94f4395c198 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -527,6 +527,7 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> u32 rsp_len;
> int rc;
> int i = 0;
> + int j;
>
> struct tpm_buf *buf __free(kfree) = kzalloc(TPM_BUFSIZE, GFP_KERNEL);
> if (!buf)
> @@ -569,6 +570,23 @@ ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip)
> pcr_select_offset = memchr_inv(pcr_selection.pcr_select, 0,
> pcr_selection.size_of_select);
> if (pcr_select_offset) {
> + for (j = 0; j < nr_alloc_banks; j++) {
> + if (chip->allocated_banks[j].alg_id == hash_alg)
> + break;
> + }
> +
> + /*
> + * The TPM protocol does not allow the same hash
> + * algorithm twice. A duplicate means this device
> + * is not behaving; do not enable it.
> + */
> + if (j != nr_alloc_banks) {
> + pr_err("tpm: duplicate PCR bank for algorithm
> 0x%04x\n",
> + hash_alg);
> + rc = -EIO;
> + break;
> + }
> +
> chip->allocated_banks[nr_alloc_banks].alg_id = hash_alg;
>
> rc = tpm2_init_bank_info(chip, nr_alloc_banks);
> --
> 2.55.0
>
Yeah, so in some places like this some of the TPM driver code does
return -EFAULT but I'd like to progressively refactor so that any
corrupted traffic is always -EIO and use -EFAULT for memory related
issues.
Thank you!
Reviewed-by: Jarkko Sakkinen <[email protected]>
Br, Jarkko
