[PATCH v2] keys/trusted_keys: reuse TPM version in option handling
Serge E. Hallyn
serge at hallyn.com
Mon Oct 5 15:01:32 UTC 2026
On Mon, Oct 05, 2026 at 11:31:46AM +0530, Srish Srinivasan wrote:
> Hi Jarkko,
>
> On 10/5/26 8:54 AM, Jarkko Sakkinen wrote:
> > On Fri, Oct 02, 2026 at 12:40:50PM +0530, Srish Srinivasan wrote:
> > > Determine the TPM version once in the seal and unseal paths and reuse it
> > > when allocating and parsing trusted key options. This avoids redundant TPM
> > > version checks.
> > So this is a bit nitpicking perhaps but a patch that would just clean
> > the code up a bit would not be be worth of applying. Almost anything
> > that "does nothing" goes to that bin.
> >
> > What I'm saying is that the conclusion in the last sentence goes to
> > wrong direction but luckily the change has also feasible effects that
> > we care about.
> >
> > By caching tpm_is_tpm2() there are less fallible sites in the key
> > creation process, and reduced number of roundtrips with the TPM chip.
> >
> > Or from blackbox perspective it reduces I/O traffic between kernel
> > and the hardware platform.
>
>
> Yes, that makes sense. Thank you for pointing this out.
>
> I will revise the commit message to highlight the reduced failure
> surface and I/O between the kernel and the TPM, and
> send v3.
And I've seen chips where every round trip is a roll of the dice as
to whether the chip will become useless till next reboot, so that is
a bigger win than I was first thinking.
> > > Signed-off-by: Srish Srinivasan <ssrish at linux.ibm.com>
> > > Reviewed-by: Stefan Berger <stefanb at linux.ibm.com>
> > > ---
> > > Changelog:
> > >
> > > v2:
> > > - Use bool to indicate whether the chip is TPM 2.0 in the option helpers
> > >
> > > security/keys/trusted-keys/trusted_tpm1.c | 35 +++++++++--------------
> > > 1 file changed, 13 insertions(+), 22 deletions(-)
> > >
> > > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > > index 3ec078ca3f97..6dbce058368f 100644
> > > --- a/security/keys/trusted-keys/trusted_tpm1.c
> > > +++ b/security/keys/trusted-keys/trusted_tpm1.c
> > > @@ -700,7 +700,7 @@ static const match_table_t key_tokens = {
> > > /* can have zero or more token= options */
> > > static int getoptions(char *c, struct trusted_key_payload *pay,
> > > - struct trusted_key_options *opt)
> > > + struct trusted_key_options *opt, bool is_tpm2)
> > > {
> > > struct trusted_key_tpm *private = opt->private;
> > > substring_t args[MAX_OPT_ARGS];
> > > @@ -712,13 +712,8 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> > > unsigned long token_mask = 0;
> > > unsigned int digest_len;
> > > int i;
> > > - int tpm2;
> > > -
> > > - tpm2 = tpm_is_tpm2(chip);
> > > - if (tpm2 < 0)
> > > - return tpm2;
> > > - private->hash = tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
> > > + private->hash = is_tpm2 ? HASH_ALGO_SHA256 : HASH_ALGO_SHA1;
> > > if (!c)
> > > return 0;
> > > @@ -773,7 +768,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> > > break;
> > > }
> > > - if (tpm2 &&
> > > + if (is_tpm2 &&
> > > private->blobauth_len <=
> > > sizeof(private->blobauth)) {
> > > memcpy(private->blobauth, args[0].from,
> > > @@ -808,14 +803,15 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> > > }
> > > if (i == HASH_ALGO__LAST)
> > > return -EINVAL;
> > > - if (!tpm2 && i != HASH_ALGO_SHA1) {
> > > + if (!is_tpm2 && i != HASH_ALGO_SHA1) {
> > > pr_info("TPM 1.x only supports SHA-1.\n");
> > > return -EINVAL;
> > > }
> > > break;
> > > case Opt_policydigest:
> > > digest_len = hash_digest_size[private->hash];
> > > - if (!tpm2 || strlen(args[0].from) != (2 * digest_len))
> > > + if (!is_tpm2 ||
> > > + strlen(args[0].from) != (2 * digest_len))
> > > return -EINVAL;
> > > res = hex2bin(private->policydigest, args[0].from,
> > > digest_len);
> > > @@ -824,7 +820,7 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> > > private->policydigest_len = digest_len;
> > > break;
> > > case Opt_policyhandle:
> > > - if (!tpm2)
> > > + if (!is_tpm2)
> > > return -EINVAL;
> > > res = kstrtoul(args[0].from, 16, &handle);
> > > if (res < 0)
> > > @@ -838,15 +834,10 @@ static int getoptions(char *c, struct trusted_key_payload *pay,
> > > return 0;
> > > }
> > > -static struct trusted_key_options *trusted_options_alloc(void)
> > > +static struct trusted_key_options *trusted_options_alloc(bool is_tpm2)
> > > {
> > > struct trusted_key_tpm *private;
> > > struct trusted_key_options *options;
> > > - int tpm2;
> > > -
> > > - tpm2 = tpm_is_tpm2(chip);
> > > - if (tpm2 < 0)
> > > - return NULL;
> > > options = kzalloc_obj(*options);
> > > if (options) {
> > > @@ -858,7 +849,7 @@ static struct trusted_key_options *trusted_options_alloc(void)
> > > kfree_sensitive(options);
> > > options = NULL;
> > > } else {
> > > - if (!tpm2)
> > > + if (!is_tpm2)
> > > private->keyhandle = SRKHANDLE;
> > > options->private = private;
> > > }
> > > @@ -877,13 +868,13 @@ static int trusted_tpm_seal(struct trusted_key_payload *p, char *datablob)
> > > if (tpm2 < 0)
> > > return tpm2;
> > > - options = trusted_options_alloc();
> > > + options = trusted_options_alloc(tpm2);
> > > if (!options)
> > > return -ENOMEM;
> > > private = options->private;
> > > - ret = getoptions(datablob, p, options);
> > > + ret = getoptions(datablob, p, options, tpm2);
> > > if (ret < 0)
> > > return ret;
> > > dump_options(options);
> > > @@ -922,12 +913,12 @@ static int trusted_tpm_unseal(struct trusted_key_payload *p, char *datablob)
> > > if (tpm2 < 0)
> > > return tpm2;
> > > - options = trusted_options_alloc();
> > > + options = trusted_options_alloc(tpm2);
> > > if (!options)
> > > return -ENOMEM;
> > > private = options->private;
> > > - ret = getoptions(datablob, p, options);
> > > + ret = getoptions(datablob, p, options, tpm2);
> > > if (ret < 0)
> > > return ret;
> > > dump_options(options);
> > > --
> > > 2.53.0
> > >
> > Br, Jarkko
>
> Thanks,
> Srish
More information about the Linux-security-module-archive
mailing list