From: "Nícolas F. R. A. Prado" <nfraprado@collabora.com>
To: James Bottomley <James.Bottomley@hansenpartnership.com>
Cc: linux-integrity@vger.kernel.org,
Jarkko Sakkinen <jarkko@kernel.org>,
keyrings@vger.kernel.org, Ard Biesheuvel <ardb@kernel.org>,
regressions@lists.linux.dev, kernel@collabora.com
Subject: Re: [PATCH v8 18/22] tpm: add session encryption protection to tpm2_get_random()
Date: Thu, 16 May 2024 20:25:54 -0400 [thread overview]
Message-ID: <119dc5ed-f159-41be-9dda-1a056f29888d@notapiano> (raw)
In-Reply-To: <20240429202811.13643-19-James.Bottomley@HansenPartnership.com>
On Mon, Apr 29, 2024 at 04:28:07PM -0400, James Bottomley wrote:
> If some entity is snooping the TPM bus, they can see the random
> numbers we're extracting from the TPM and do prediction attacks
> against their consumers. Foil this attack by using response
> encryption to prevent the attacker from seeing the random sequence.
>
> Signed-off-by: James Bottomley <James.Bottomley@HansenPartnership.com>
> Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
>
> ---
> v7: add review
> ---
> drivers/char/tpm/tpm2-cmd.c | 21 +++++++++++++++++----
> 1 file changed, 17 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index a53a843294ed..0cdf892ec2a7 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -292,25 +292,35 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> if (!num_bytes || max > TPM_MAX_RNG_DATA)
> return -EINVAL;
>
> - err = tpm_buf_init(&buf, 0, 0);
> + err = tpm2_start_auth_session(chip);
> if (err)
> return err;
>
> + err = tpm_buf_init(&buf, 0, 0);
> + if (err) {
> + tpm2_end_auth_session(chip);
> + return err;
> + }
> +
> do {
> - tpm_buf_reset(&buf, TPM2_ST_NO_SESSIONS, TPM2_CC_GET_RANDOM);
> + tpm_buf_reset(&buf, TPM2_ST_SESSIONS, TPM2_CC_GET_RANDOM);
> + tpm_buf_append_hmac_session_opt(chip, &buf, TPM2_SA_ENCRYPT
> + | TPM2_SA_CONTINUE_SESSION,
> + NULL, 0);
> tpm_buf_append_u16(&buf, num_bytes);
> + tpm_buf_fill_hmac_session(chip, &buf);
> err = tpm_transmit_cmd(chip, &buf,
> offsetof(struct tpm2_get_random_out,
> buffer),
> "attempting get random");
> + err = tpm_buf_check_hmac_response(chip, &buf, err);
> if (err) {
> if (err > 0)
> err = -EIO;
> goto out;
> }
>
> - out = (struct tpm2_get_random_out *)
> - &buf.data[TPM_HEADER_SIZE];
> + out = (struct tpm2_get_random_out *)tpm_buf_parameters(&buf);
> recd = min_t(u32, be16_to_cpu(out->size), num_bytes);
> if (tpm_buf_length(&buf) <
> TPM_HEADER_SIZE +
> @@ -327,9 +337,12 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
> } while (retries-- && total < max);
>
> tpm_buf_destroy(&buf);
> + tpm2_end_auth_session(chip);
> +
> return total ? total : -EIO;
> out:
> tpm_buf_destroy(&buf);
> + tpm2_end_auth_session(chip);
> return err;
> }
>
> --
> 2.35.3
>
Hi,
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)
The warning is the following:
[ 2.017338] ------------[ cut here ]------------
[ 2.025521] WARNING: CPU: 0 PID: 72 at kernel/module/kmod.c:144 __request_module+0x188/0x1f4
[ 2.039508] Modules linked in:
[ 2.046447] CPU: 0 PID: 72 Comm: kworker/u34:3 Not tainted 6.9.0 #1
[ 2.046455] Hardware name: Google juniper sku16 board (DT)
[ 2.046460] Workqueue: async async_run_entry_fn
[ 2.060094]
[ 2.060097] pstate: 00000005 (nzcv daif -PAN -UAO -TCO -DIT -SSBS BTYPE=--)
[ 2.091758] pc : __request_module+0x188/0x1f4
[ 2.096114] lr : __request_module+0x180/0x1f4
[ 2.100468] sp : ffff80008088b400
[ 2.103777] x29: ffff80008088b400 x28: 0000000000281ae0 x27: ffffa13fd366e0d2
[ 2.110915] x26: 0000000000000000 x25: ffff2387008f33c0 x24: 00000000ffffffff
[ 2.118053] x23: 000000000000200f x22: ffffa13fd0ed49de x21: 0000000000000001
[ 2.125190] x20: 0000000000000000 x19: ffffa13fd23f0be0 x18: 0000000000000014
[ 2.132327] x17: 00000000fbdae5e3 x16: 000000005bcbb9f8 x15: 000000008700f694
[ 2.139463] x14: 0000000000000001 x13: ffff80008088b850 x12: 0000000000000000
[ 2.146600] x11: 00000000f8f4a4bb x10: fffffffffdd186ae x9 : 0000000000000004
[ 2.153736] x8 : ffff2387008f33c0 x7 : 3135616873286361 x6 : 0c0406065b07370f
[ 2.160873] x5 : 0f37075b0606040c x4 : 0000000000000000 x3 : 0000000000000000
[ 2.168009] x2 : ffffa13fd0ed49de x1 : ffffa13fcfcc4768 x0 : 0000000000000001
[ 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
[ 2.222632] tpm2_start_auth_session+0xc8/0x29c
[ 2.227162] tpm2_get_random+0x44/0x204
[ 2.230996] tpm_get_random+0x74/0x90
[ 2.234655] tpm_hwrng_read+0x24/0x30
[ 2.238314] add_early_randomness+0x68/0x118
[ 2.242584] hwrng_register+0x16c/0x218
[ 2.246418] tpm_chip_register+0xf0/0x2cc
[ 2.248143] cros-ec-spi spi2.0: SPI transfer timed out
[ 2.250419] tpm_tis_core_init+0x494/0x7e0
[ 2.255552] spi_master spi2: failed to transfer one message from queue
[ 2.259623] tpm_tis_spi_init+0x54/0x70
[ 2.259629] cr50_spi_probe+0xf4/0x27c
[ 2.266145] spi_master spi2: noqueue transfer failed
[ 2.269961] tpm_tis_spi_driver_probe+0x34/0x64
[ 2.273704] cros-ec-spi spi2.0: spi transfer failed: -110
[ 2.278647] spi_probe+0x84/0xe4
[ 2.291799] really_probe+0xbc/0x2a0
[ 2.295373] __driver_probe_device+0x78/0x12c
[ 2.299725] driver_probe_device+0xdc/0x160
[ 2.303903] __device_attach_driver+0xb8/0x134
[ 2.308342] bus_for_each_drv+0x84/0xe0
[ 2.312174] __device_attach_async_helper+0xac/0xd0
[ 2.317051] async_run_entry_fn+0x34/0xe0
[ 2.321058] process_one_work+0x150/0x294
[ 2.325068] worker_thread+0x304/0x408
[ 2.328816] kthread+0x118/0x11c
[ 2.332045] ret_from_fork+0x10/0x20
[ 2.335621] ---[ end trace 0000000000000000 ]---
Which is generated in __request_module() here:
/*
* We don't allow synchronous module loading from async. Module
* init may invoke async_synchronize_full() which will end up
* waiting for this task which already is waiting for the module
* loading to complete, leading to a deadlock.
*/
WARN_ON_ONCE(wait && current_is_async());
As the comment says this could lead to a deadlock so it seemed worthwhile to
report and get fixed.
The tpm_tis_spi driver probes asynchronously,
.probe_type = PROBE_PREFER_ASYNCHRONOUS,
and as part of its probe registers the tpm device which ultimately leads to a
module being requested synchronously in crypto_larval_lookup():
request_module("crypto-%s-all", name);
and that triggers the warning.
#regzbot introduced: 1b6d7f9eb150
#regzbot title: __request_module warning: sync module loading from async tpm driver probe
Thanks,
Nícolas
next prev parent reply other threads:[~2024-05-17 0:25 UTC|newest]
Thread overview: 75+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-04-29 20:27 [PATCH v8 00/22] add integrity and security to TPM2 transactions James Bottomley
2024-04-29 20:27 ` [PATCH v8 01/22] tpm: Remove unused tpm_buf_tag() James Bottomley
2024-04-29 20:27 ` [PATCH v8 02/22] tpm: Remove tpm_send() James Bottomley
2024-04-29 20:27 ` [PATCH v8 03/22] tpm: Move buffer handling from static inlines to real functions James Bottomley
2024-04-29 20:27 ` [PATCH v8 04/22] tpm: Update struct tpm_buf documentation comments James Bottomley
2024-04-29 20:27 ` [PATCH v8 05/22] tpm: Store the length of the tpm_buf data separately James Bottomley
2024-04-29 20:27 ` [PATCH v8 06/22] tpm: TPM2B formatted buffers James Bottomley
2024-04-29 20:27 ` [PATCH v8 07/22] tpm: Add tpm_buf_read_{u8,u16,u32} James Bottomley
2024-04-29 20:27 ` [PATCH v8 08/22] KEYS: trusted: tpm2: Use struct tpm_buf for sized buffers James Bottomley
2024-04-29 20:27 ` [PATCH v8 09/22] crypto: lib - implement library version of AES in CFB mode James Bottomley
2024-04-29 20:27 ` [PATCH v8 10/22] tpm: add buffer function to point to returned parameters James Bottomley
2024-04-29 20:28 ` [PATCH v8 11/22] tpm: export the context save and load commands James Bottomley
2024-04-29 20:28 ` [PATCH v8 12/22] tpm: Add NULL primary creation James Bottomley
2024-04-29 22:37 ` Jarkko Sakkinen
2024-04-29 20:28 ` [PATCH v8 13/22] tpm: Add TCG mandated Key Derivation Functions (KDFs) James Bottomley
2024-04-29 22:37 ` Jarkko Sakkinen
2024-04-29 20:28 ` [PATCH v8 14/22] tpm: Add HMAC session start and end functions James Bottomley
2024-04-29 22:38 ` Jarkko Sakkinen
2024-04-30 16:49 ` Jarkko Sakkinen
2024-04-29 20:28 ` [PATCH v8 15/22] tpm: Add HMAC session name/handle append James Bottomley
2024-04-29 22:38 ` Jarkko Sakkinen
2024-04-29 20:28 ` [PATCH v8 16/22] tpm: Add the rest of the session HMAC API James Bottomley
2024-04-29 22:39 ` Jarkko Sakkinen
2024-04-29 20:28 ` [PATCH v8 17/22] tpm: add hmac checks to tpm2_pcr_extend() James Bottomley
2024-04-29 20:28 ` [PATCH v8 18/22] tpm: add session encryption protection to tpm2_get_random() James Bottomley
2024-05-17 0:25 ` Nícolas F. R. A. Prado [this message]
2024-05-17 1:59 ` James Bottomley
2024-05-17 7:20 ` Ard Biesheuvel
2024-05-17 8:26 ` Jarkko Sakkinen
2024-05-17 13:35 ` James Bottomley
2024-05-17 13:43 ` Ard Biesheuvel
2024-05-17 14:25 ` James Bottomley
2024-05-17 16:22 ` Nícolas F. R. A. Prado
2024-05-17 16:48 ` Jarkko Sakkinen
2024-05-18 4:31 ` Eric Biggers
2024-05-18 7:03 ` [PATCH] crypto: api - Do not load modules until algapi is ready Herbert Xu
2024-05-18 11:04 ` Jarkko Sakkinen
2024-05-18 12:32 ` Herbert Xu
2024-05-18 13:03 ` Jarkko Sakkinen
2024-05-18 13:07 ` James Bottomley
2024-05-19 4:19 ` Herbert Xu
2024-05-20 15:49 ` Nícolas F. R. A. Prado
2024-05-21 2:53 ` [v2 PATCH] crypto: api - Do not load modules if called by async probing Herbert Xu
2024-05-21 19:37 ` Nícolas F. R. A. Prado
2024-05-22 5:37 ` [v3 PATCH] hwrng: core - Remove add_early_randomness Herbert Xu
2024-05-22 11:51 ` Jarkko Sakkinen
2024-05-23 4:50 ` Herbert Xu
2024-05-22 19:19 ` Nícolas F. R. A. Prado
2024-05-22 22:53 ` Linus Torvalds
2024-05-23 4:49 ` Herbert Xu
2024-05-23 9:53 ` Jarkko Sakkinen
2024-05-23 9:58 ` Herbert Xu
2024-05-23 10:07 ` Jarkko Sakkinen
2024-05-23 10:02 ` Jarkko Sakkinen
2024-05-23 10:40 ` Torsten Duwe
2024-05-18 10:56 ` [PATCH v8 18/22] tpm: add session encryption protection to tpm2_get_random() Jarkko Sakkinen
2024-05-18 12:31 ` Herbert Xu
2024-04-29 20:28 ` [PATCH v8 19/22] KEYS: trusted: Add session encryption protection to the seal/unseal path James Bottomley
2024-04-29 20:28 ` [PATCH v8 20/22] tpm: add the null key name as a sysfs export James Bottomley
2024-04-29 20:28 ` [PATCH v8 21/22] Documentation: add tpm-security.rst James Bottomley
2024-04-29 20:28 ` [PATCH v8 22/22] tpm: disable the TPM if NULL name changes James Bottomley
2024-04-29 22:59 ` Jarkko Sakkinen
2024-04-29 23:34 ` Jarkko Sakkinen
2024-04-29 22:22 ` [PATCH v8 00/22] add integrity and security to TPM2 transactions Jarkko Sakkinen
2024-04-29 22:26 ` Jarkko Sakkinen
2024-04-29 23:49 ` Jarkko Sakkinen
2024-04-30 11:18 ` Stefan Berger
2024-04-30 18:37 ` Jarkko Sakkinen
2024-04-30 18:57 ` Stefan Berger
2024-04-30 19:23 ` James Bottomley
2024-04-30 21:48 ` Jarkko Sakkinen
2024-04-30 22:31 ` James Bottomley
2024-04-30 22:46 ` Jarkko Sakkinen
2024-04-30 23:10 ` Jarkko Sakkinen
2024-05-03 23:18 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=119dc5ed-f159-41be-9dda-1a056f29888d@notapiano \
--to=nfraprado@collabora.com \
--cc=James.Bottomley@hansenpartnership.com \
--cc=ardb@kernel.org \
--cc=jarkko@kernel.org \
--cc=kernel@collabora.com \
--cc=keyrings@vger.kernel.org \
--cc=linux-integrity@vger.kernel.org \
--cc=regressions@lists.linux.dev \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.