On Wed, Sep 30, 2026 at 12:25:06AM +0300, Jarkko Sakkinen wrote:
> 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]>
WARNING: Reported-by: should be immediately followed by Closes: or Link: with a
URL to the report
#18:
Reported-by: Vega <[email protected]>
Assisted-by: LLM
No need to resend a new version but I'd need a link to the *report* to
retain reported-by tag here.
Br, Jarkko
