Re: [PATCH v8 18/22] tpm: add session encryption protection to tpm2_get_random()

["Jarkko Sakkinen" <jarkko@kernel.org>]

On Fri May 17, 2024 at 10:20 AM EEST, Ard Biesheuvel wrote:
> On Fri, 17 May 2024 at 03:59, James Bottomley
> <James.Bottomley@hansenpartnership.com> wrote:
> >
> > On Thu, 2024-05-16 at 20:25 -0400, Nícolas F. R. A. Prado wrote:
> ...
> > > KernelCI has identified a new warning and I tracked it down to this
> > > commit. It
> > > was observed on the following platforms:
> > > * mt8183-kukui-jacuzzi-juniper-sku16
> > > * sc7180-trogdor-kingoftown
> > > (but probably affects all platforms that have a tpm driver with async
> > > probe)
> > >
> > > [    2.175146] Call trace:
> > > [    2.177587]  __request_module+0x188/0x1f4
> > > [    2.181596]  crypto_alg_mod_lookup+0x178/0x21c
> > > [    2.186042]  crypto_alloc_tfm_node+0x58/0x114
> > > [    2.190396]  crypto_alloc_shash+0x24/0x30
> > > [    2.194404]  drbg_init_hash_kernel+0x28/0xdc
> > > [    2.198673]  drbg_kcapi_seed+0x21c/0x420
> > > [    2.202593]  crypto_rng_reset+0x84/0xb4
> > > [    2.206425]  crypto_get_default_rng+0xa4/0xd8
> > > [    2.210779]  ecc_gen_privkey+0x58/0xd0
> > > [    2.214526]  ecdh_set_secret+0x90/0x198
> > > [    2.218360]  tpm_buf_append_salt+0x164/0x2dc
> >
> > This looks like a misconfiguration.  The kernel is trying to load the
> > ecdh module, but it should have been selected as built in by this in
> > drivers/char/tpm/Kconfig:
> >
> > config TCG_TPM2_HMAC
> >         bool "Use HMAC and encrypted transactions on the TPM bus"
> >         default y
> >         select CRYPTO_ECDH
> >         select CRYPTO_LIB_AESCFB
> >         select CRYPTO_LIB_SHA256
> >
>
> The module request is not for ECDH itself but for the DRBG it attempts
> to use to generate the secret.
>
> Given that CRYPTO_ECDH does not strictly require a DRBG in principle,
> but does in this particular case, I think it makes sense to select
> CRYPTO_DRBG here (or depend on it being builtin), rather than updating
> the Kconfig rules for CRYPTO_ECDH itself.

I can spin a new PR if James can make a fix.

All previous 4 PR's for 6.10 were applied to Linus' tree so my queue
is empty. Need to have both fixes and stable-tags to save my bandwidth.

Maybe for transcript just two first lines denoting that it was
__request_module() will do. That and adding CONFIG_DRBG will take
it away should be enough for the full disclosure, right?

BR, Jarkko