hwrng framework does not have a requirement that the all bytes requested
need to be provided. By enforcing such a requirement internally, TPM driver
can cause unpredictability in latency, as a single tpm_get_random() call
can result multiple TPM commands.
Especially, when TCG_TPM2_HMAC is enabled, extra roundtrips could have
significant effect to the system latency.
Thus, send TPM command only once and return bytes received instead of
committing to the number of requested bytes.
Cc: David S. Miller <davem@davemloft.net>
Cc: Herbert Xu <herbert@gondor.apana.org.au>
Cc: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v7:
- Given that hwrng is now only caller for tpm_get_random(), remove the
wait parameter.
v4:
- Fixed grammar mistakes.
---
drivers/char/tpm/tpm-interface.c | 28 +++++-----------------------
1 file changed, 5 insertions(+), 23 deletions(-)
diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
index d157be738612..677dcef05dfb 100644
--- a/drivers/char/tpm/tpm-interface.c
+++ b/drivers/char/tpm/tpm-interface.c
@@ -626,10 +626,6 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
*/
int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
{
- u32 num_bytes = max;
- u8 *out_ptr = out;
- int retries = 5;
- int total = 0;
int rc;
if (!out || !max || max > TPM_MAX_RNG_DATA)
@@ -646,28 +642,14 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
rc = tpm2_start_auth_session(chip);
if (rc)
return rc;
- }
-
- do {
- if (chip->flags & TPM_CHIP_FLAG_TPM2)
- rc = tpm2_get_random(chip, out_ptr, num_bytes);
- else
- rc = tpm1_get_random(chip, out_ptr, num_bytes);
-
- if (rc < 0)
- goto err;
-
- out_ptr += rc;
- total += rc;
- num_bytes -= rc;
- } while (retries-- && total < max);
- tpm_put_ops(chip);
- return total ? total : -EIO;
+ rc = tpm2_get_random(chip, out, max);
+ } else {
+ rc = tpm1_get_random(chip, out, max);
+ }
-err:
tpm_put_ops(chip);
- return rc;
+ return rc != 0 ? rc : -EIO;
}
EXPORT_SYMBOL_GPL(tpm_get_random);
--
2.39.5
On Tue, Dec 16, 2025 at 11:21:38AM +0200, Jarkko Sakkinen wrote:
>hwrng framework does not have a requirement that the all bytes requested
>need to be provided. By enforcing such a requirement internally, TPM driver
>can cause unpredictability in latency, as a single tpm_get_random() call
>can result multiple TPM commands.
>
>Especially, when TCG_TPM2_HMAC is enabled, extra roundtrips could have
>significant effect to the system latency.
>
>Thus, send TPM command only once and return bytes received instead of
>committing to the number of requested bytes.
Function comment for tpm_get_random needs updated as well, as it
currently says "until all of the @max bytes have been received", which
is no longer true with this patch. With that:
Reviewed-by: Jonathan McDowell <noodles@meta.com>
>Cc: David S. Miller <davem@davemloft.net>
>Cc: Herbert Xu <herbert@gondor.apana.org.au>
>Cc: Eric Biggers <ebiggers@kernel.org>
>Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
>---
>v7:
>- Given that hwrng is now only caller for tpm_get_random(), remove the
> wait parameter.
>v4:
>- Fixed grammar mistakes.
>---
> drivers/char/tpm/tpm-interface.c | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
>diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
>index d157be738612..677dcef05dfb 100644
>--- a/drivers/char/tpm/tpm-interface.c
>+++ b/drivers/char/tpm/tpm-interface.c
>@@ -626,10 +626,6 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> */
> int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> {
>- u32 num_bytes = max;
>- u8 *out_ptr = out;
>- int retries = 5;
>- int total = 0;
> int rc;
>
> if (!out || !max || max > TPM_MAX_RNG_DATA)
>@@ -646,28 +642,14 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> rc = tpm2_start_auth_session(chip);
> if (rc)
> return rc;
>- }
>-
>- do {
>- if (chip->flags & TPM_CHIP_FLAG_TPM2)
>- rc = tpm2_get_random(chip, out_ptr, num_bytes);
>- else
>- rc = tpm1_get_random(chip, out_ptr, num_bytes);
>-
>- if (rc < 0)
>- goto err;
>-
>- out_ptr += rc;
>- total += rc;
>- num_bytes -= rc;
>- } while (retries-- && total < max);
>
>- tpm_put_ops(chip);
>- return total ? total : -EIO;
>+ rc = tpm2_get_random(chip, out, max);
>+ } else {
>+ rc = tpm1_get_random(chip, out, max);
>+ }
>
>-err:
> tpm_put_ops(chip);
>- return rc;
>+ return rc != 0 ? rc : -EIO;
> }
> EXPORT_SYMBOL_GPL(tpm_get_random);
>
>--
>2.39.5
>
>
J.
--
... We are talking one charming motherf**king pig.
On Fri, Dec 19, 2025 at 09:42:07AM +0000, Jonathan McDowell wrote: > On Tue, Dec 16, 2025 at 11:21:38AM +0200, Jarkko Sakkinen wrote: > > hwrng framework does not have a requirement that the all bytes requested > > need to be provided. By enforcing such a requirement internally, TPM driver > > can cause unpredictability in latency, as a single tpm_get_random() call > > can result multiple TPM commands. > > > > Especially, when TCG_TPM2_HMAC is enabled, extra roundtrips could have > > significant effect to the system latency. > > > > Thus, send TPM command only once and return bytes received instead of > > committing to the number of requested bytes. > > Function comment for tpm_get_random needs updated as well, as it currently > says "until all of the @max bytes have been received", which is no longer > true with this patch. With that: > > Reviewed-by: Jonathan McDowell <noodles@meta.com> Thank you and definitely can refine that comment. After holidays it is probably to go through this patch set with time and send +1 iteration :-) BR, Jarkko
On Tue, Dec 16, 2025 at 11:21:38AM +0200, Jarkko Sakkinen wrote:
> hwrng framework does not have a requirement that the all bytes requested
> need to be provided. By enforcing such a requirement internally, TPM driver
> can cause unpredictability in latency, as a single tpm_get_random() call
> can result multiple TPM commands.
>
> Especially, when TCG_TPM2_HMAC is enabled, extra roundtrips could have
> significant effect to the system latency.
>
> Thus, send TPM command only once and return bytes received instead of
> committing to the number of requested bytes.
>
> Cc: David S. Miller <davem@davemloft.net>
> Cc: Herbert Xu <herbert@gondor.apana.org.au>
> Cc: Eric Biggers <ebiggers@kernel.org>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> ---
> v7:
> - Given that hwrng is now only caller for tpm_get_random(), remove the
> wait parameter.
> v4:
> - Fixed grammar mistakes.
> ---
IMHO, these first four patches that add sanity to tpm_get_random() usage
ought to be somewhat "dead obvious" category.
Once these are merged it is much easier to further reduce the frequency
of tpm_get_random() calls made, and test their effect on e.g. call
frequency (as tpm_get_random() is now much more tighly bound to hwrng
requirements).
> drivers/char/tpm/tpm-interface.c | 28 +++++-----------------------
> 1 file changed, 5 insertions(+), 23 deletions(-)
>
> diff --git a/drivers/char/tpm/tpm-interface.c b/drivers/char/tpm/tpm-interface.c
> index d157be738612..677dcef05dfb 100644
> --- a/drivers/char/tpm/tpm-interface.c
> +++ b/drivers/char/tpm/tpm-interface.c
> @@ -626,10 +626,6 @@ static int tpm2_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> */
> int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> {
> - u32 num_bytes = max;
> - u8 *out_ptr = out;
> - int retries = 5;
> - int total = 0;
> int rc;
>
> if (!out || !max || max > TPM_MAX_RNG_DATA)
> @@ -646,28 +642,14 @@ int tpm_get_random(struct tpm_chip *chip, u8 *out, size_t max)
> rc = tpm2_start_auth_session(chip);
> if (rc)
> return rc;
> - }
> -
> - do {
> - if (chip->flags & TPM_CHIP_FLAG_TPM2)
> - rc = tpm2_get_random(chip, out_ptr, num_bytes);
> - else
> - rc = tpm1_get_random(chip, out_ptr, num_bytes);
> -
> - if (rc < 0)
> - goto err;
> -
> - out_ptr += rc;
> - total += rc;
> - num_bytes -= rc;
> - } while (retries-- && total < max);
>
> - tpm_put_ops(chip);
> - return total ? total : -EIO;
> + rc = tpm2_get_random(chip, out, max);
> + } else {
> + rc = tpm1_get_random(chip, out, max);
> + }
>
> -err:
> tpm_put_ops(chip);
> - return rc;
> + return rc != 0 ? rc : -EIO;
> }
> EXPORT_SYMBOL_GPL(tpm_get_random);
>
> --
> 2.39.5
>
BR, Jarkko
© 2016 - 2026 Red Hat, Inc.