linux-kernel - Re: [PATCH v7 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <c0e0d549-a41f-4803-b4c2-b31ed877f4cf@linux.ibm.com>
Date: Wed, 23 Oct 2024 15:28:06 -0400
From: Stefan Berger <stefanb@...ux.ibm.com>
To: Jarkko Sakkinen <jarkko@...nel.org>, linux-integrity@...r.kernel.org,
        Peter Huewe <peterhuewe@....de>, Jason Gunthorpe <jgg@...pe.ca>
Cc: David Howells <dhowells@...hat.com>,
        James Bottomley <James.Bottomley@...senPartnership.com>,
        Mimi Zohar <zohar@...ux.ibm.com>,
        Roberto Sassu <roberto.sassu@...wei.com>, linux-kernel@...r.kernel.org,
        Pengyu Ma <mapengyu@...il.com>, stable@...r.kernel.org
Subject: Re: [PATCH v7 5/5] tpm: flush the auth session only when /dev/tpm0 is
 open



On 10/21/24 1:39 AM, 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);

That could also go into the inline. Looks good otherwise.

>   			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 4eaa8e05c291..a3ed7a99a394 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);
>   		chip->null_key = 0;
>   	}
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index bfa47d48b0f2..2363018fa8fb 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -381,6 +381,7 @@ int tpm_pm_suspend(struct device *dev)
>   	if (!rc) {
>   		if (chip->flags & TPM_CHIP_FLAG_TPM2) {
>   #ifdef CONFIG_TCG_TPM2_HMAC
> +			tpm2_end_auth_session(chip);
>   			tpm2_flush_context(chip, chip->null_key);
>   			chip->null_key = 0;
>   #endif
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index 6e52785de9fd..a7079c7ec6d1 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -333,6 +333,9 @@ void tpm_buf_append_hmac_session(struct tpm_chip *chip, struct tpm_buf *buf,
>   	}
>   
>   #ifdef CONFIG_TCG_TPM2_HMAC
> +	/* The first write to /dev/tpm{rm0} will flush the session. */
> +	attributes |= TPM2_SA_CONTINUE_SESSION;
> +
>   	/*
>   	 * The Architecture Guide requires us to strip trailing zeros
>   	 * before computing the HMAC