On Mon, Oct 26, 2026 at 01:11:21AM +0200, Jarkko Sakkinen wrote:
> On Sun, Oct 25, 2026 at 03:47:23PM -0700, Eric Biggers wrote:
> > On Mon, Oct 26, 2026 at 12:12:13AM +0200, Jarkko Sakkinen wrote:
> > > On Sun, Oct 25, 2026 at 02:11:19PM -0700, Eric Biggers wrote:
> > > > crypto_kpp_set_secret() can fail, for example if the kernel is booted
> > > > with fips=1 and the broken-by-design FIPS RNG used for ECC private key
> > > > generation doesn't feel like generating random bytes at the moment.
> > > > Check for error, cancelling the TPM session creation rather than
> > > > proceeding with a zero or uninitialized private key.
> > > >
> > > > Fixes: 699e3efd6c64 ("tpm: Add HMAC session start and end functions")
> > > > Cc: [email protected]
> > > > Signed-off-by: Eric Biggers <[email protected]>
> > > > ---
> > > > drivers/char/tpm/tpm2-sessions.c | 4 +++-
> > > > 1 file changed, 3 insertions(+), 1 deletion(-)
> > > >
> > > > diff --git a/drivers/char/tpm/tpm2-sessions.c
> > > > b/drivers/char/tpm/tpm2-sessions.c
> > > > index ca1e2bf424e14..2aaaad69760dc 100644
> > > > --- a/drivers/char/tpm/tpm2-sessions.c
> > > > +++ b/drivers/char/tpm/tpm2-sessions.c
> > > > @@ -527,7 +527,9 @@ static int tpm_buf_append_salt(struct tpm_buf *buf,
> > > > struct tpm_chip *chip,
> > > > }
> > > > crypto_ecdh_encode_key(encoded_key, buf_len, &p);
> > > > /* this generates a random private key */
> > >
> > > Could you delete this comment while at it? It's not helping with
> > > anything.
> > >
> > > > - crypto_kpp_set_secret(kpp, encoded_key, buf_len);
> > > > + rc = crypto_kpp_set_secret(kpp, encoded_key, buf_len);
> > > > + if (rc)
> > > > + goto err_free_kpp;
> > > >
> > > > /* salt is now the public point of this private key */
> > > > req = kpp_request_alloc(kpp, GFP_KERNEL);
> > > >
> >
> > I actually think the comment is helpful, because of how unreadable the
> > crypto_kpp API is. It's generating a private key, but to request that,
> > crypto_kpp apparently requires that an empty key be encoded into a byte
> > string. Then that key is "set", which actually generates a new key.
> >
> > In comparison, the Curve25519 equivalent is
> > curve25519_generate_secret(), which is clearly named and also always
> > succeeds. At some point we need to fix the ECDH API...
>
> Ah, right it gets zero filled echd. I've not consciously thought this
> detail before, thanks :-)
>
> Reviewed-by: Jarkko Sakkinen <[email protected]>
> >
> > - Eric
>
> tpm2-sessions overall would need some refurbishing work now that also
> Matthew's hibernate patches depend on it. I have to take a serious
> look at it some point.
I'll apply this after Linus has accepted/rejected my TPM pull request.
I'm not sure if it already is but if it isn't, probably also at
linux-integrity turning on Sashiko would make sense.
Br, Jarkko
