Re: [PATCH] keys/trusted_keys: reuse TPM version in option handling

[Srish Srinivasan]

Hi Jarkko,
Thanks for taking a look.

On 10/2/26 9:01 AM, Jarkko Sakkinen wrote:
> On Wed, Sep 30, 2026 at 11:32:17AM +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.
> >
> > Signed-off-by: Srish Srinivasan <ssrish@xxxxxxxxxxxxx>
> > Reviewed-by: Stefan Berger <stefanb@xxxxxxxxxxxxx>
> > ---
> >   security/keys/trusted-keys/trusted_tpm1.c | 22 ++++++----------------
> >   1 file changed, 6 insertions(+), 16 deletions(-)
> >
> > diff --git a/security/keys/trusted-keys/trusted_tpm1.c b/security/keys/trusted-keys/trusted_tpm1.c
> > index 3ec078ca3f97..70de4101923f 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, int tpm2)
> >   {
> >   	struct trusted_key_tpm *private = opt->private;
> >   	substring_t args[MAX_OPT_ARGS];
> > @@ -712,11 +712,6 @@ 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;
> >   
> > @@ -838,15 +833,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(int tpm2)
> Probably "bool is_tpm2" would be better here. It does not need to be
> tightly coupled with the tristate acquired from tpm_is_tpm2.


Makes sense.
Sure, I will make this change and send out v2.


> >   {
> >   	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) {
> > @@ -877,13 +867,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 +912,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.