[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <aMxaKleVR8i6s9vX@kernel.org>
Date: Thu, 18 Sep 2025 22:14:50 +0300
From: Jarkko Sakkinen <jarkko@...nel.org>
To: linux-integrity@...r.kernel.org
Cc: Frédéric Jouen <fjouen@...lsq.com>,
Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>,
James Bottomley <James.Bottomley@...senpartnership.com>,
Mimi Zohar <zohar@...ux.ibm.com>,
David Howells <dhowells@...hat.com>,
Paul Moore <paul@...l-moore.com>, James Morris <jmorris@...ei.org>,
"Serge E. Hallyn" <serge@...lyn.com>,
open list <linux-kernel@...r.kernel.org>,
"open list:KEYS-TRUSTED" <keyrings@...r.kernel.org>,
"open list:SECURITY SUBSYSTEM" <linux-security-module@...r.kernel.org>
Subject: Re: [PATCH] tpm: use a map for tpm2_calc_ordinal_duration()
On Thu, Sep 18, 2025 at 09:57:30PM +0300, Jarkko Sakkinen wrote:
> The current shenanigans for duration calculation introduce too much
> complexity for a trivial problem, and further the code is hard to patch and
> maintain.
>
> Address these issues with a flat look-up table, which is easy to understand
> and patch. If leaf driver specific patching is required in future, it is
> easy enough to make a copy of this table during driver initialization and
> add the chip parameter back.
>
> 'chip->duration' is retained for TPM 1.x.
>
> As the first entry for this new behavior address TCG spec update mentioned
> in this issue:
>
> https://github.com/raspberrypi/linux/issues/7054
>
> Therefore, for TPM_SelfTest the duration is set to 3000 ms.
>
> This does not categorize a as bug, given that this is introduced to the
> spec after the feature was originally made.
>
> Cc: Frédéric Jouen <fjouen@...lsq.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>
> ---
> drivers/char/tpm/tpm-interface.c | 2 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/char/tpm/tpm2-cmd.c | 115 +++++++++----------------------
> 3 files changed, 34 insertions(+), 85 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index b71725827743..c9f173001d0e 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -52,7 +52,7 @@ MODULE_PARM_DESC(suspend_pcr,
> unsigned long tpm_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
> {
> if (chip->flags & TPM_CHIP_FLAG_TPM2)
> - return tpm2_calc_ordinal_duration(chip, ordinal);
> + return tpm2_calc_ordinal_duration(ordinal);
> else
> return tpm1_calc_ordinal_duration(chip, ordinal);
> }
> diff --git a/drivers/char/tpm/tpm.h b/drivers/char/tpm/tpm.h
> index 7bb87fa5f7a1..2726bd38e5ac 100644
> --- a/drivers/char/tpm/tpm.h
> +++ b/drivers/char/tpm/tpm.h
> @@ -299,7 +299,7 @@ ssize_t tpm2_get_tpm_pt(struct tpm_chip *chip, u32 property_id,
> ssize_t tpm2_get_pcr_allocation(struct tpm_chip *chip);
> int tpm2_auto_startup(struct tpm_chip *chip);
> void tpm2_shutdown(struct tpm_chip *chip, u16 shutdown_type);
> -unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal);
> +unsigned long tpm2_calc_ordinal_duration(u32 ordinal);
> int tpm2_probe(struct tpm_chip *chip);
> int tpm2_get_cc_attrs_tbl(struct tpm_chip *chip);
> int tpm2_find_cc(struct tpm_chip *chip, u32 cc);
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index 524d802ede26..29c0d6a8ec20 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -48,100 +48,49 @@ int tpm2_get_timeouts(struct tpm_chip *chip)
> return 0;
> }
>
> -/**
> - * tpm2_ordinal_duration_index() - returns an index to the chip duration table
> - * @ordinal: TPM command ordinal.
> - *
> - * The function returns an index to the chip duration table
> - * (enum tpm_duration), that describes the maximum amount of
> - * time the chip could take to return the result for a particular ordinal.
> - *
> - * The values of the MEDIUM, and LONG durations are taken
> - * from the PC Client Profile (PTP) specification (750, 2000 msec)
> - *
> - * LONG_LONG is for commands that generates keys which empirically takes
> - * a longer time on some systems.
> - *
> - * Return:
> - * * TPM_MEDIUM
> - * * TPM_LONG
> - * * TPM_LONG_LONG
> - * * TPM_UNDEFINED
> +/*
> + * Contains the maximum durations in milliseconds for TPM2 commands.
> */
> -static u8 tpm2_ordinal_duration_index(u32 ordinal)
> -{
> - switch (ordinal) {
> - /* Startup */
> - case TPM2_CC_STARTUP: /* 144 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_SELF_TEST: /* 143 */
> - return TPM_LONG;
> -
> - case TPM2_CC_GET_RANDOM: /* 17B */
> - return TPM_LONG;
> -
> - case TPM2_CC_SEQUENCE_UPDATE: /* 15C */
> - return TPM_MEDIUM;
> - case TPM2_CC_SEQUENCE_COMPLETE: /* 13E */
> - return TPM_MEDIUM;
> - case TPM2_CC_EVENT_SEQUENCE_COMPLETE: /* 185 */
> - return TPM_MEDIUM;
> - case TPM2_CC_HASH_SEQUENCE_START: /* 186 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_VERIFY_SIGNATURE: /* 177 */
> - return TPM_LONG_LONG;
> -
> - case TPM2_CC_PCR_EXTEND: /* 182 */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_HIERARCHY_CONTROL: /* 121 */
> - return TPM_LONG;
> - case TPM2_CC_HIERARCHY_CHANGE_AUTH: /* 129 */
> - return TPM_LONG;
> -
> - case TPM2_CC_GET_CAPABILITY: /* 17A */
> - return TPM_MEDIUM;
> -
> - case TPM2_CC_NV_READ: /* 14E */
> - return TPM_LONG;
> -
> - case TPM2_CC_CREATE_PRIMARY: /* 131 */
> - return TPM_LONG_LONG;
> - case TPM2_CC_CREATE: /* 153 */
> - return TPM_LONG_LONG;
> - case TPM2_CC_CREATE_LOADED: /* 191 */
> - return TPM_LONG_LONG;
> -
> - default:
> - return TPM_UNDEFINED;
> - }
> -}
> +static const struct {
> + unsigned long ordinal;
> + unsigned long duration;
> +} tpm2_ordinal_duration_map[] = {
> + {TPM2_CC_STARTUP, 750},
> + {TPM2_CC_SELF_TEST, 3000},
> + {TPM2_CC_GET_RANDOM, 2000},
> + {TPM2_CC_SEQUENCE_UPDATE, 750},
> + {TPM2_CC_SEQUENCE_COMPLETE, 750},
> + {TPM2_CC_EVENT_SEQUENCE_COMPLETE, 750},
> + {TPM2_CC_HASH_SEQUENCE_START, 750},
> + {TPM2_CC_VERIFY_SIGNATURE, 30000},
> + {TPM2_CC_PCR_EXTEND, 750},
> + {TPM2_CC_HIERARCHY_CONTROL, 2000},
> + {TPM2_CC_HIERARCHY_CHANGE_AUTH, 2000},
> + {TPM2_CC_GET_CAPABILITY, 750},
> + {TPM2_CC_NV_READ, 2000},
> + {TPM2_CC_CREATE_PRIMARY, 30000},
> + {TPM2_CC_CREATE, 30000},
> + {TPM2_CC_CREATE_LOADED, 30000},
> +};
>
> /**
> - * tpm2_calc_ordinal_duration() - calculate the maximum command duration
> - * @chip: TPM chip to use.
> + * tpm2_calc_ordinal_duration() - Calculate the maximum command duration
> * @ordinal: TPM command ordinal.
> *
> - * The function returns the maximum amount of time the chip could take
> - * to return the result for a particular ordinal in jiffies.
> - *
> - * Return: A maximal duration time for an ordinal in jiffies.
> + * Returns the maximum amount of time the chip is expected by kernel to
> + * take in jiffies.
> */
> -unsigned long tpm2_calc_ordinal_duration(struct tpm_chip *chip, u32 ordinal)
> +unsigned long tpm2_calc_ordinal_duration(u32 ordinal)
> {
> - unsigned int index;
> + int i;
>
> - index = tpm2_ordinal_duration_index(ordinal);
> + for (i = 0; i < ARRAY_SIZE(tpm2_ordinal_duration_map); i++)
> + if (ordinal == tpm2_ordinal_duration_map[i].ordinal)
> + return tpm2_ordinal_duration_map[i].duration;
>
> - if (index != TPM_UNDEFINED)
> - return chip->duration[index];
> - else
> - return msecs_to_jiffies(TPM2_DURATION_DEFAULT);
> + return TPM2_DURATION_DEFAULT;
Ouch, I'm fully acknowledged that msecs_to_jiffies() is missing from the patch.
I'll post +1 so that it is testable.
> }
>
> -
> struct tpm2_pcr_read_out {
> __be32 update_cnt;
> __be32 pcr_selects_cnt;
> --
> 2.39.5
>
>
BR, Jarkko
Powered by blists - more mailing lists