* [PATCH v2] keys/trusted_keys: reuse TPM version in option handling
@ 2026-10-02 7:10 Srish Srinivasan
2026-10-05 3:24 ` Jarkko Sakkinen
0 siblings, 1 reply; 6+ messages in thread
From: Srish Srinivasan @ 2026-10-02 7:10 UTC (permalink / raw)
To: linux-integrity, keyrings
Cc: James.Bottomley, jarkko, zohar, stefanb, linux-kernel,
linux-security-module, nayna, rnsastry, ssrish
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>
---
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] keys/trusted_keys: reuse TPM version in option handling
2026-10-02 7:10 [PATCH v2] keys/trusted_keys: reuse TPM version in option handling Srish Srinivasan
@ 2026-10-05 3:24 ` Jarkko Sakkinen
2026-10-05 6:01 ` Srish Srinivasan
0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2026-10-05 3:24 UTC (permalink / raw)
To: Srish Srinivasan
Cc: linux-integrity, keyrings, James.Bottomley, jarkko, zohar,
stefanb, linux-kernel, linux-security-module, nayna, rnsastry
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.
>
> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
> Reviewed-by: Stefan Berger <stefanb@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] keys/trusted_keys: reuse TPM version in option handling
2026-10-05 3:24 ` Jarkko Sakkinen
@ 2026-10-05 6:01 ` Srish Srinivasan
2026-10-05 15:01 ` Serge E. Hallyn
0 siblings, 1 reply; 6+ messages in thread
From: Srish Srinivasan @ 2026-10-05 6:01 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: linux-integrity, keyrings, James.Bottomley, jarkko, zohar,
stefanb, linux-kernel, linux-security-module, nayna, rnsastry
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.
>
>> Signed-off-by: Srish Srinivasan <ssrish@linux.ibm.com>
>> Reviewed-by: Stefan Berger <stefanb@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] keys/trusted_keys: reuse TPM version in option handling
2026-10-05 6:01 ` Srish Srinivasan
@ 2026-10-05 15:01 ` Serge E. Hallyn
2026-10-05 19:38 ` Jarkko Sakkinen
0 siblings, 1 reply; 6+ messages in thread
From: Serge E. Hallyn @ 2026-10-05 15:01 UTC (permalink / raw)
To: Srish Srinivasan
Cc: Jarkko Sakkinen, linux-integrity, keyrings, James.Bottomley,
jarkko, zohar, stefanb, linux-kernel, linux-security-module,
nayna, rnsastry
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@linux.ibm.com>
> > > Reviewed-by: Stefan Berger <stefanb@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
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] keys/trusted_keys: reuse TPM version in option handling
2026-10-05 15:01 ` Serge E. Hallyn
@ 2026-10-05 19:38 ` Jarkko Sakkinen
2026-10-05 21:57 ` Jarkko Sakkinen
0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2026-10-05 19:38 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Srish Srinivasan, Jarkko Sakkinen, linux-integrity, keyrings,
James.Bottomley, zohar, stefanb, linux-kernel,
linux-security-module, nayna, rnsastry
On Mon, Oct 05, 2026 at 10:01:32AM -0500, Serge E. Hallyn wrote:
> 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.
This is true and happens quite often. And in CPU companies in fact all
the time with early FPGA implementations and very early ASIC based
software development platforms.
We also have opt-in bus encryption feature where extra roundtrips have
a horrible time cost.
Roundtrip removals are high value patches for both R&D and production,
and they are always most welcome.
Br, Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2] keys/trusted_keys: reuse TPM version in option handling
2026-10-05 19:38 ` Jarkko Sakkinen
@ 2026-10-05 21:57 ` Jarkko Sakkinen
0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2026-10-05 21:57 UTC (permalink / raw)
To: Serge E. Hallyn
Cc: Srish Srinivasan, linux-integrity, keyrings, James.Bottomley,
zohar, stefanb, linux-kernel, linux-security-module, nayna,
rnsastry
On Mon, Oct 05, 2026 at 10:38:32PM +0300, Jarkko Sakkinen wrote:
> On Mon, Oct 05, 2026 at 10:01:32AM -0500, Serge E. Hallyn wrote:
> > 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.
>
> This is true and happens quite often. And in CPU companies in fact all
> the time with early FPGA implementations and very early ASIC based
> software development platforms.
>
> We also have opt-in bus encryption feature where extra roundtrips have
> a horrible time cost.
>
> Roundtrip removals are high value patches for both R&D and production,
> and they are always most welcome.
Serge, your comment reminded me of hwrng. In my opinion TPM driver
should limit itself randomness supply because it could have positive
effect on some issues that people are having [1]. hwrng can never
properly control TPM as it is behind abstraction.
I think there should be a tpm_chip associated pool of randomness. This
pool is filled with some timer triggering with a constant frequency. On
each iteration, it is filled with best effort i.e. with a single
TPM2_GetRandom invocation.
Then hwrng_fill is also filled with best effort from the pool without
any TPM interaction.
This would be an architecture that would be vastly easier to deduce
than the one which we have now, which is more unstable, which is
bad considering e.g., latency peaks and whatnot.
This basic idea can be improved with a low watermark: when going
below it hwrng_fillfn would start the timer. I don't think high
watermark is needed as pool can be always filled just up to its
size.
I had this idea already 3-4 years ago but had forgotten about it.
[1] https://bugzilla.kernel.org/show_bug.cgi?id=217890
Br, Jarkkko
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-05 21:58 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-02 7:10 [PATCH v2] keys/trusted_keys: reuse TPM version in option handling Srish Srinivasan
2026-10-05 3:24 ` Jarkko Sakkinen
2026-10-05 6:01 ` Srish Srinivasan
2026-10-05 15:01 ` Serge E. Hallyn
2026-10-05 19:38 ` Jarkko Sakkinen
2026-10-05 21:57 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®