[PATCH] tpm: Fix barriers to prevent hwrng from activating during resume

[PATCH] tpm: Fix barriers to prevent hwrng from activating during resume

[Thomas Fourier]

The memory barrier is placed after clearing the suspened flag so
memory reordering between the wake up and clearing the flag is allowed.
The reader of the flag is missing the corresponding barrier.

Clear the flag after the memory barrier and add a memory barrier in
the reader.

Fixes: 99d464506255 ("tpm: Prevent hwrng from activating during resume")
Cc: <stable@vger.kernel.org>
Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
---
 drivers/char/tpm/tpm-chip.c      | 3 +++
 drivers/char/tpm/tpm-interface.c | 3 +--
 2 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
index 12b7394b34bd..7f500797b7a7 100644
--- a/drivers/char/tpm/tpm-chip.c
+++ b/drivers/char/tpm/tpm-chip.c
@@ -173,6 +173,9 @@ int tpm_try_get_ops(struct tpm_chip *chip)
 	if (chip->flags & TPM_CHIP_FLAG_SUSPENDED)
 		goto out_lock;
 
+	/* Ensure that device is fully resumed */
+	rmb();
+
 	rc = tpm_chip_start(chip);
 	if (rc)
 		goto out_lock;
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index f745a098908b..2de12b02f62b 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -474,13 +474,12 @@ int tpm_pm_resume(struct device *dev)
 	if (chip == NULL)
 		return -ENODEV;
 
-	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
-
 	/*
 	 * Guarantee that SUSPENDED is written last, so that hwrng does not
 	 * activate before the chip has been fully resumed.
 	 */
 	wmb();
+	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
 
 	return 0;
 }
-- 
2.43.0

Re: [PATCH] tpm: Fix barriers to prevent hwrng from activating during resume

[Jarkko Sakkinen]

On Mon, Aug 03, 2026 at 11:13:24AM +0200, Thomas Fourier wrote:
> The memory barrier is placed after clearing the suspened flag so
> memory reordering between the wake up and clearing the flag is allowed.
> The reader of the flag is missing the corresponding barrier.
> 
> Clear the flag after the memory barrier and add a memory barrier in
> the reader.
> 
> Fixes: 99d464506255 ("tpm: Prevent hwrng from activating during resume")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>

Thanks I investigate this properly before answering. It's my first day
after holiday so might take until Tue/Wed.

PS. And if someone noticed, sorry for copy-pasting the same response to
two bug reports. I just cherry-picked critical looking stuff from my
inbox :-)

BR, Jarkko

> ---
>  drivers/char/tpm/tpm-chip.c      | 3 +++
>  drivers/char/tpm/tpm-interface.c | 3 +--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 12b7394b34bd..7f500797b7a7 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -173,6 +173,9 @@ int tpm_try_get_ops(struct tpm_chip *chip)
>  	if (chip->flags & TPM_CHIP_FLAG_SUSPENDED)
>  		goto out_lock;
>  
> +	/* Ensure that device is fully resumed */
> +	rmb();
> +
>  	rc = tpm_chip_start(chip);
>  	if (rc)
>  		goto out_lock;
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index f745a098908b..2de12b02f62b 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -474,13 +474,12 @@ int tpm_pm_resume(struct device *dev)
>  	if (chip == NULL)
>  		return -ENODEV;
>  
> -	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
> -
>  	/*
>  	 * Guarantee that SUSPENDED is written last, so that hwrng does not
>  	 * activate before the chip has been fully resumed.
>  	 */
>  	wmb();
> +	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
>  
>  	return 0;
>  }
> -- 
> 2.43.0
> 

Re: [PATCH] tpm: Fix barriers to prevent hwrng from activating during resume

[Jarkko Sakkinen]

On Mon, Aug 03, 2026 at 11:13:24AM +0200, Thomas Fourier wrote:
> The memory barrier is placed after clearing the suspened flag so
> memory reordering between the wake up and clearing the flag is allowed.
> The reader of the flag is missing the corresponding barrier.
> 
> Clear the flag after the memory barrier and add a memory barrier in
> the reader.
> 
> Fixes: 99d464506255 ("tpm: Prevent hwrng from activating during resume")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
> ---
>  drivers/char/tpm/tpm-chip.c      | 3 +++
>  drivers/char/tpm/tpm-interface.c | 3 +--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 12b7394b34bd..7f500797b7a7 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -173,6 +173,9 @@ int tpm_try_get_ops(struct tpm_chip *chip)
>  	if (chip->flags & TPM_CHIP_FLAG_SUSPENDED)
>  		goto out_lock;
>  
> +	/* Ensure that device is fully resumed */
> +	rmb();
> +
>  	rc = tpm_chip_start(chip);
>  	if (rc)
>  		goto out_lock;
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index f745a098908b..2de12b02f62b 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -474,13 +474,12 @@ int tpm_pm_resume(struct device *dev)
>  	if (chip == NULL)
>  		return -ENODEV;
>  
> -	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
> -
>  	/*
>  	 * Guarantee that SUSPENDED is written last, so that hwrng does not
>  	 * activate before the chip has been fully resumed.
>  	 */
>  	wmb();
> +	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
>  
>  	return 0;
>  }
> -- 
> 2.43.0


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Applied.

BR, Jarkko

Re: [PATCH] tpm: Fix barriers to prevent hwrng from activating during resume

[Jarkko Sakkinen]

On Mon, Aug 03, 2026 at 11:13:24AM +0200, Thomas Fourier wrote:
> The memory barrier is placed after clearing the suspened flag so
> memory reordering between the wake up and clearing the flag is allowed.
> The reader of the flag is missing the corresponding barrier.
> 
> Clear the flag after the memory barrier and add a memory barrier in
> the reader.
> 
> Fixes: 99d464506255 ("tpm: Prevent hwrng from activating during resume")
> Cc: <stable@vger.kernel.org>
> Signed-off-by: Thomas Fourier <fourier.thomas@gmail.com>
> ---

I tagged this too quickly, sorry, let's revisit the patch a bit
just to make sure that we do right things right.

>  drivers/char/tpm/tpm-chip.c      | 3 +++
>  drivers/char/tpm/tpm-interface.c | 3 +--
>  2 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
> index 12b7394b34bd..7f500797b7a7 100644
> --- a/drivers/char/tpm/tpm-chip.c
> +++ b/drivers/char/tpm/tpm-chip.c
> @@ -173,6 +173,9 @@ int tpm_try_get_ops(struct tpm_chip *chip)
>  	if (chip->flags & TPM_CHIP_FLAG_SUSPENDED)
>  		goto out_lock;
>  
> +	/* Ensure that device is fully resumed */
> +	rmb();
> +
>  	rc = tpm_chip_start(chip);
>  	if (rc)
>  		goto out_lock;
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index f745a098908b..2de12b02f62b 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -474,13 +474,12 @@ int tpm_pm_resume(struct device *dev)
>  	if (chip == NULL)
>  		return -ENODEV;
>  
> -	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
> -
>  	/*
>  	 * Guarantee that SUSPENDED is written last, so that hwrng does not
>  	 * activate before the chip has been fully resumed.
>  	 */
>  	wmb();
> +	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;

Can you rationalize this change?

>  
>  	return 0;
>  }
> -- 
> 2.43.0
> 

BR, Jarkko

Re: [PATCH] tpm: Fix barriers to prevent hwrng from activating during resume

[Richard Lyu]

>> memory reordering between the wake up and clearing the flag is allowed.

When exactly can this reordering happen?

>> diff --git a/drivers/char/tpm/tpm-chip.c b/drivers/char/tpm/tpm-chip.c
>> index 12b7394b34bd..7f500797b7a7 100644 --- a/drivers/char/tpm/tpm-chip.c
>> +++ b/drivers/char/tpm/tpm-chip.c @@ -173,6 +173,9 @@ int
>> tpm_try_get_ops(struct tpm_chip *chip) if (chip->flags &
>> TPM_CHIP_FLAG_SUSPENDED) goto out_lock;
>>
>> +	/* Ensure that device is fully resumed */
>> +	rmb();
>> +
>>  	rc = tpm_chip_start(chip);
>>  	if (rc)
>>  		goto out_lock;

Where inside tpm_chip_start do we actually need to avoid loading a 
stale state or flag?

>> diff --git a/drivers/char/tpm/tpm-interface.c
>> b/drivers/char/tpm/tpm-interface.c index f745a098908b..2de12b02f62b 100644
>> --- a/drivers/char/tpm/tpm-interface.c +++
>> b/drivers/char/tpm/tpm-interface.c @@ -474,13 +474,12 @@ int
>> tpm_pm_resume(struct device *dev) if (chip == NULL) return -ENODEV;
>>
>> -	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
>> -
>>  	/*
>>  	 * Guarantee that SUSPENDED is written last, so that hwrng does not
>>  	 * activate before the chip has been fully resumed.
>>  	 */
>>  	wmb();
>> +	chip->flags &= ~TPM_CHIP_FLAG_SUSPENDED;
>
>Can you rationalize this change?

I agree the clearing of the flag should be moved after wmb() to guarantee 
that SUSPENDED is written last, the flag has to be cleared after the barrier.
That part makes sense.
  
My remaining question is whether we actually need the wmb() barrier here?