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

[Srish Srinivasan <ssrish@linux.ibm.com>]

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@linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb@linux.ibm.com>
>> ---
>>   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.