The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH] tpm: tpm_tis_spi: Use wait_woken() in wait_for_tmp_stat()
@ 2026-05-09 18:51 Jarkko Sakkinen
  2026-05-10 20:45 ` Stefan Wahren
  2026-05-11  8:10 ` Linus Walleij
  0 siblings, 2 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2026-05-09 18:51 UTC (permalink / raw)
  To: linux-integrity
  Cc: Jarkko Sakkinen, stable, Linus Walleij, Stefan Wahren,
	Peter Huewe, Jason Gunthorpe, linux-kernel

wait_event_interruptible_timeout() evaluates its condition after setting
the current task state to TASK_INTERRUPTIBLE.

With CONFIG_DEBUG_ATOMIC_SLEEP this triggers a warning when the IRQ wait
path is used:

    tpm_tis_status()
      tpm_tis_spi_read_bytes()
        tpm_tis_spi_transfer_full()
          spi_bus_lock()
            mutex_lock()

Address this with the following measures:

1. Call wait_tpm_stat_cond() only while tasking is running.
2. Use wait_woken() to wait for changes.

Cc: stable@vger.kernel.org # v4.19+
Cc: Linus Walleij <linusw@kernel.org>
Reported-by: Stefan Wahren <wahrenst@gmx.net>
Closes: https://lore.kernel.org/linux-integrity/6964bec7-3dbb-453b-89ef-9b990217a8b9@gmx.net/
Fixes: 1a339b658d9d ("tpm_tis_spi: Pass the SPI IRQ down to the driver")
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
Linus' change only unmasked a pre-existing bug but it is the change
realizes it in tpm_tis_spi.
 drivers/char/tpm/tpm_tis_core.c | 35 ++++++++++++++++++++-------------
 1 file changed, 21 insertions(+), 14 deletions(-)

diff --git a/drivers/char/tpm/tpm_tis_core.c b/drivers/char/tpm/tpm_tis_core.c
index 21d79ad3b164..153a57c79240 100644
--- a/drivers/char/tpm/tpm_tis_core.c
+++ b/drivers/char/tpm/tpm_tis_core.c
@@ -66,8 +66,8 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
 		bool check_cancel)
 {
 	struct tpm_tis_data *priv = dev_get_drvdata(&chip->dev);
+	DEFINE_WAIT_FUNC(wait, woken_wake_function);
 	unsigned long stop;
-	long rc;
 	u8 status;
 	bool canceled = false;
 	u8 sts_mask;
@@ -87,23 +87,30 @@ static int wait_for_tpm_stat(struct tpm_chip *chip, u8 mask,
 	/* process status changes with irq support */
 	if (sts_mask) {
 		ret = -ETIME;
+		add_wait_queue(queue, &wait);
 again:
+		if (wait_for_tpm_stat_cond(chip, sts_mask, check_cancel,
+					   &canceled)) {
+			ret = canceled ? -ECANCELED : 0;
+			goto out;
+		}
+
 		timeout = stop - jiffies;
 		if ((long)timeout <= 0)
-			return -ETIME;
-		rc = wait_event_interruptible_timeout(*queue,
-			wait_for_tpm_stat_cond(chip, sts_mask, check_cancel,
-					       &canceled),
-			timeout);
-		if (rc > 0) {
-			if (canceled)
-				return -ECANCELED;
-			ret = 0;
-		}
-		if (rc == -ERESTARTSYS && freezing(current)) {
-			clear_thread_flag(TIF_SIGPENDING);
-			goto again;
+			goto out;
+
+		if (signal_pending(current)) {
+			if (freezing(current)) {
+				clear_thread_flag(TIF_SIGPENDING);
+				goto again;
+			}
+			goto out;
 		}
+
+		wait_woken(&wait, TASK_INTERRUPTIBLE, timeout);
+		goto again;
+out:
+		remove_wait_queue(queue, &wait);
 	}

 	if (ret)
--
2.47.3


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] tpm: tpm_tis_spi: Use wait_woken() in wait_for_tmp_stat()
  2026-05-09 18:51 [PATCH] tpm: tpm_tis_spi: Use wait_woken() in wait_for_tmp_stat() Jarkko Sakkinen
@ 2026-05-10 20:45 ` Stefan Wahren
  2026-05-11  8:10 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Stefan Wahren @ 2026-05-10 20:45 UTC (permalink / raw)
  To: Jarkko Sakkinen, linux-integrity
  Cc: stable, Linus Walleij, Peter Huewe, Jason Gunthorpe, linux-kernel

Am 09.05.26 um 20:51 schrieb Jarkko Sakkinen:
> wait_event_interruptible_timeout() evaluates its condition after setting
> the current task state to TASK_INTERRUPTIBLE.
>
> With CONFIG_DEBUG_ATOMIC_SLEEP this triggers a warning when the IRQ wait
> path is used:
>
>      tpm_tis_status()
>        tpm_tis_spi_read_bytes()
>          tpm_tis_spi_transfer_full()
>            spi_bus_lock()
>              mutex_lock()
>
> Address this with the following measures:
>
> 1. Call wait_tpm_stat_cond() only while tasking is running.
> 2. Use wait_woken() to wait for changes.
>
> Cc: stable@vger.kernel.org # v4.19+
> Cc: Linus Walleij <linusw@kernel.org>
> Reported-by: Stefan Wahren <wahrenst@gmx.net>
> Closes: https://lore.kernel.org/linux-integrity/6964bec7-3dbb-453b-89ef-9b990217a8b9@gmx.net/
> Fixes: 1a339b658d9d ("tpm_tis_spi: Pass the SPI IRQ down to the driver")
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>
The issue isn't reproducible anymore. Thanks

Tested-by: Stefan Wahren <wahrenst@gmx.net>

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] tpm: tpm_tis_spi: Use wait_woken() in wait_for_tmp_stat()
  2026-05-09 18:51 [PATCH] tpm: tpm_tis_spi: Use wait_woken() in wait_for_tmp_stat() Jarkko Sakkinen
  2026-05-10 20:45 ` Stefan Wahren
@ 2026-05-11  8:10 ` Linus Walleij
  1 sibling, 0 replies; 3+ messages in thread
From: Linus Walleij @ 2026-05-11  8:10 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: linux-integrity, stable, Stefan Wahren, Peter Huewe,
	Jason Gunthorpe, linux-kernel

On Sat, May 9, 2026 at 8:51 PM Jarkko Sakkinen <jarkko@kernel.org> wrote:

> wait_event_interruptible_timeout() evaluates its condition after setting
> the current task state to TASK_INTERRUPTIBLE.
>
> With CONFIG_DEBUG_ATOMIC_SLEEP this triggers a warning when the IRQ wait
> path is used:
>
>     tpm_tis_status()
>       tpm_tis_spi_read_bytes()
>         tpm_tis_spi_transfer_full()
>           spi_bus_lock()
>             mutex_lock()
>
> Address this with the following measures:
>
> 1. Call wait_tpm_stat_cond() only while tasking is running.
> 2. Use wait_woken() to wait for changes.
>
> Cc: stable@vger.kernel.org # v4.19+
> Cc: Linus Walleij <linusw@kernel.org>
> Reported-by: Stefan Wahren <wahrenst@gmx.net>
> Closes: https://lore.kernel.org/linux-integrity/6964bec7-3dbb-453b-89ef-9b990217a8b9@gmx.net/
> Fixes: 1a339b658d9d ("tpm_tis_spi: Pass the SPI IRQ down to the driver")
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> Linus' change only unmasked a pre-existing bug but it is the change
> realizes it in tpm_tis_spi.

Took me a while to understand this but looks right to me!
Reviewed-by: Linus Walleij <linusw@kernel.org>

Yours,
Linus Walleij

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2026-05-11  8:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-09 18:51 [PATCH] tpm: tpm_tis_spi: Use wait_woken() in wait_for_tmp_stat() Jarkko Sakkinen
2026-05-10 20:45 ` Stefan Wahren
2026-05-11  8:10 ` Linus Walleij

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox