All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jonathan McDowell <noodles@earth.li>
To: "Orlov, Ivan" <iorlov@amazon.co.uk>
Cc: "peterhuewe@gmx.de" <peterhuewe@gmx.de>,
	"jarkko@kernel.org" <jarkko@kernel.org>,
	"jgg@ziepe.ca" <jgg@ziepe.ca>,
	"linux-integrity@vger.kernel.org"
	<linux-integrity@vger.kernel.org>,
	"linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
	"Woodhouse, David" <dwmw@amazon.co.uk>
Subject: Re: [PATCH] tpm: Fix the timeout & use ktime
Date: Fri, 20 Jun 2025 14:24:28 +0100	[thread overview]
Message-ID: <aFVhDDewVHneFXnO@earth.li> (raw)
In-Reply-To: <20250611162508.85149-1-iorlov@amazon.com>

On Wed, Jun 11, 2025 at 04:25:24PM +0000, Orlov, Ivan wrote:
>The current implementation of timeout detection works in the following
>way:
>
>1. Read completion status. If completed, return the data
>2. Sleep for some time (usleep_range)
>3. Check for timeout using current jiffies value. Return an error if
>   timed out
>4. Goto 1
>
>usleep_range doesn't guarantee it's always going to wake up strictly in
>(min, max) range, so such a situation is possible:
>
>1. Driver reads completion status. No completion yet
>2. Process sleeps indefinitely. In the meantime, TPM responds
>3. We check for timeout without checking for the completion again.
>   Result is lost.

This looks similar to the issue I fixed in 7146dffa875c ('Fix timeout
handling when waiting for TPM status'), I assume you're actually seeing
it in your systems? I think we're starting to see it (rarely) now the
other issues are fixed in our builds. As a similar approach does the
following work?

diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index 8d7e4da6ed53..18ae0767fa60 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -127,7 +127,7 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
  		goto out_recv;
  
  	stop = jiffies + tpm_calc_ordinal_duration(chip, ordinal);
-	do {
+	while (true) {
  		u8 status = tpm_chip_status(chip);
  		if ((status & chip->ops->req_complete_mask) ==
  		    chip->ops->req_complete_val)
@@ -138,9 +138,12 @@ static ssize_t tpm_try_transmit(struct tpm_chip *chip, void *buf, size_t bufsiz)
  			return -ECANCELED;
  		}
  
+		if (time_after(jiffies, stop))
+			break;
+
  		tpm_msleep(TPM_TIMEOUT_POLL);
  		rmb();
-	} while (time_before(jiffies, stop));
+	}
  
  	tpm_chip_cancel(chip);
  	dev_err(&chip->dev, "Operation Timed out\n");

  parent reply	other threads:[~2025-06-20 14:03 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-11 16:25 [PATCH] tpm: Fix the timeout & use ktime Orlov, Ivan
2025-06-11 17:02 ` Jarkko Sakkinen
2025-06-20 17:19   ` Orlov, Ivan
2025-06-20 13:24 ` Jonathan McDowell [this message]
2025-06-20 17:23   ` Orlov, Ivan

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=aFVhDDewVHneFXnO@earth.li \
    --to=noodles@earth.li \
    --cc=dwmw@amazon.co.uk \
    --cc=iorlov@amazon.co.uk \
    --cc=jarkko@kernel.org \
    --cc=jgg@ziepe.ca \
    --cc=linux-integrity@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=peterhuewe@gmx.de \
    /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.