linux-kernel - Re: [PATCH v6 5/5] tpm: flush the auth session only when /dev/tpm0 is open

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-Id: <D501OQWL1TT4.24C0QEV958D75@kernel.org>
Date: Sat, 19 Oct 2024 22:55:02 +0300
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "Jarkko Sakkinen" <jarkko@...nel.org>, "Peter Huewe"
 <peterhuewe@....de>, "Jason Gunthorpe" <jgg@...pe.ca>
Cc: "Stefan Berger" <stefanb@...ux.ibm.com>, "Pengyu Ma"
 <mapengyu@...il.com>, <stable@...r.kernel.org>,
 <linux-integrity@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v6 5/5] tpm: flush the auth session only when /dev/tpm0
 is open

On Tue Oct 15, 2024 at 11:58 PM EEST, Jarkko Sakkinen wrote:
> Instead of flushing and reloading the auth session for every single
> transaction, keep the session open unless /dev/tpm0 is used. In practice
> this means applying TPM2_SA_CONTINUE_SESSION to the session attributes.
> Flush the session always when /dev/tpm0 is written.
>
> Reported-by: Pengyu Ma <mapengyu@...il.com>
> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=219229
> Cc: stable@...r.kernel.org # v6.10+
> Fixes: 7ca110f2679b ("tpm: Address !chip->auth in tpm_buf_append_hmac_session*()")
> Tested-by: Pengyu Ma <mapengyu@...il.com>
> Signed-off-by: Jarkko Sakkinen <jarkko@...nel.org>
> ---
> v5:
> - No changes.
> v4:
> - Changed as bug.
> v3:
> - Refined the commit message.
> - Removed the conditional for applying TPM2_SA_CONTINUE_SESSION only when
>   /dev/tpm0 is open. It is not required as the auth session is flushed,
>   not saved.
> v2:
> - A new patch.
> ---
>  drivers/char/tpm/tpm-chip.c       | 1 +
>  drivers/char/tpm/tpm-dev-common.c | 1 +
>  drivers/char/tpm/tpm-interface.c  | 1 +
>  drivers/char/tpm/tpm2-sessions.c  | 3 +++
>  4 files changed, 6 insertions(+)
>
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 0ea00e32f575..7a6bb30d1f32 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -680,6 +680,7 @@ void tpm_chip_unregister(struct tpm_chip *chip)
>  	rc = tpm_try_get_ops(chip);
>  	if (!rc) {
>  		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> +			tpm2_end_auth_session(chip);
>  			tpm2_flush_context(chip, chip->null_key);
>  			chip->null_key = 0;
>  		}
> diff --git a/drivers/char/tpm/tpm-dev-common.c b/drivers/char/tpm/tpm-dev-common.c
> index 4bc07963e260..c6fdeb4feaef 100644
> --- a/drivers/char/tpm/tpm-dev-common.c
> +++ b/drivers/char/tpm/tpm-dev-common.c
> @@ -29,6 +29,7 @@ static ssize_t tpm_dev_transmit(struct tpm_chip *chip, struct tpm_space *space,
>  
>  #ifdef CONFIG_TCG_TPM2_HMAC
>  	if (chip->flags & TPM_CHIP_FLAG_TPM2) {
> +		tpm2_end_auth_session(chip);
>  		tpm2_flush_context(chip, chip->null_key);

The reporter has done already too much so unless someone is willing to
verify these with matching hardware specs patch by patch I'm not into
meking any changes. It makes the flow factors better still what it used
to be and final result is not messy. It is good enough in my books and
performance fixes are sensitive.

BR, Jarkko