public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] tpm: Drop warning when an auth session is active
       [not found] ` <Z8oV9lJ4hsHualcP@kernel.org>
@ 2025-03-07 10:56   ` Jonathan McDowell
  2025-03-07 16:36     ` Jarkko Sakkinen
  2025-03-07 10:58   ` [PATCH] tpm: Lazily flush auth session when getting random data Jonathan McDowell
  1 sibling, 1 reply; 6+ messages in thread
From: Jonathan McDowell @ 2025-03-07 10:56 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe
  Cc: James Bottomley, Mimi Zohar, linux-integrity, linux-kernel

Auth sessions are lazily flushed since commit df745e25098dc ("tpm:
Lazily flush the auth session"), so it's expected that we might try to
start a new session when one is still active.

Signed-off-by: Jonathan McDowell <noodles@meta.com>
---
 drivers/char/tpm/tpm2-sessions.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
index b70165b588ec..2d2c192ebb14 100644
--- a/drivers/char/tpm/tpm2-sessions.c
+++ b/drivers/char/tpm/tpm2-sessions.c
@@ -982,7 +982,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
 	int rc;
 
 	if (chip->auth) {
-		dev_warn_once(&chip->dev, "auth session is active\n");
 		return 0;
 	}
 
-- 
2.48.1


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

* [PATCH] tpm: Lazily flush auth session when getting random data
       [not found] ` <Z8oV9lJ4hsHualcP@kernel.org>
  2025-03-07 10:56   ` [PATCH] tpm: Drop warning when an auth session is active Jonathan McDowell
@ 2025-03-07 10:58   ` Jonathan McDowell
  2025-03-07 16:34     ` Jarkko Sakkinen
  1 sibling, 1 reply; 6+ messages in thread
From: Jonathan McDowell @ 2025-03-07 10:58 UTC (permalink / raw)
  To: Jarkko Sakkinen, Peter Huewe, Jason Gunthorpe
  Cc: James Bottomley, linux-integrity, linux-kernel

From: Jonathan McDowell <noodles@meta.com>

Lazy flushing of TPM auth sessions was introduced to speed up IMA
measurments into the TPM. Make use of it in tpm2_get_random as well,
which has the added benefit of not needlessly cleaning up the session
that IMA is using when there are no userspace accesses taking place.

Command trace before for every call:

hwrng (0x00000161): 14 (52965242 ns)
hwrng (0x00000176): 48 (161612432 ns)
hwrng (0x00000165): 10 (2410494 ns)
hwrng (0x0000017B): 117 (70699883 ns)
hwrng (0x0000017B): 117 (70959666 ns)
hwrng (0x00000165): 10 (2756827 ns)

After, with repeated calls showing no setup:

hwrng (0x00000161): 14 (53044582 ns)
hwrng (0x00000176): 48 (160491333 ns)
hwrng (0x00000165): 10 (2408220 ns)
hwrng (0x0000017B): 117 (70695037 ns)
hwrng (0x0000017B): 117 (70994984 ns)
hwrng (0x0000017B): 117 (70195388 ns)
hwrng (0x0000017B): 117 (70973835 ns)

Signed-off-by: Jonathan McDowell <noodles@meta.com>
---
 drivers/char/tpm/tpm2-cmd.c | 1 -
 1 file changed, 1 deletion(-)

diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
index dfdcbd009720..524d802ede26 100644
--- a/drivers/char/tpm/tpm2-cmd.c
+++ b/drivers/char/tpm/tpm2-cmd.c
@@ -359,7 +359,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
 	} while (retries-- && total < max);
 
 	tpm_buf_destroy(&buf);
-	tpm2_end_auth_session(chip);
 
 	return total ? total : -EIO;
 out:
-- 
2.48.1


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

* Re: [PATCH] tpm: Lazily flush auth session when getting random data
  2025-03-07 10:58   ` [PATCH] tpm: Lazily flush auth session when getting random data Jonathan McDowell
@ 2025-03-07 16:34     ` Jarkko Sakkinen
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-03-07 16:34 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: Peter Huewe, Jason Gunthorpe, James Bottomley, linux-integrity,
	linux-kernel

On Fri, Mar 07, 2025 at 10:58:13AM +0000, Jonathan McDowell wrote:
> From: Jonathan McDowell <noodles@meta.com>
> 
> Lazy flushing of TPM auth sessions was introduced to speed up IMA
> measurments into the TPM. Make use of it in tpm2_get_random as well,
> which has the added benefit of not needlessly cleaning up the session
> that IMA is using when there are no userspace accesses taking place.
> 
> Command trace before for every call:
> 
> hwrng (0x00000161): 14 (52965242 ns)
> hwrng (0x00000176): 48 (161612432 ns)
> hwrng (0x00000165): 10 (2410494 ns)
> hwrng (0x0000017B): 117 (70699883 ns)
> hwrng (0x0000017B): 117 (70959666 ns)
> hwrng (0x00000165): 10 (2756827 ns)
> 
> After, with repeated calls showing no setup:
> 
> hwrng (0x00000161): 14 (53044582 ns)
> hwrng (0x00000176): 48 (160491333 ns)
> hwrng (0x00000165): 10 (2408220 ns)
> hwrng (0x0000017B): 117 (70695037 ns)
> hwrng (0x0000017B): 117 (70994984 ns)
> hwrng (0x0000017B): 117 (70195388 ns)
> hwrng (0x0000017B): 117 (70973835 ns)
> 
> Signed-off-by: Jonathan McDowell <noodles@meta.com>
> ---
>  drivers/char/tpm/tpm2-cmd.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm2-cmd.c b/drivers/char/tpm/tpm2-cmd.c
> index dfdcbd009720..524d802ede26 100644
> --- a/drivers/char/tpm/tpm2-cmd.c
> +++ b/drivers/char/tpm/tpm2-cmd.c
> @@ -359,7 +359,6 @@ int tpm2_get_random(struct tpm_chip *chip, u8 *dest, size_t max)
>  	} while (retries-- && total < max);
>  
>  	tpm_buf_destroy(&buf);
> -	tpm2_end_auth_session(chip);
>  
>  	return total ? total : -EIO;
>  out:
> -- 
> 2.48.1
> 

Thanks for this. It is a good catch! I'll apply this over the weekend
when I apply Arm FF-A driver patches.

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

BR, Jarkko

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

* Re: [PATCH] tpm: Drop warning when an auth session is active
  2025-03-07 10:56   ` [PATCH] tpm: Drop warning when an auth session is active Jonathan McDowell
@ 2025-03-07 16:36     ` Jarkko Sakkinen
  2025-03-07 17:25       ` Jonathan McDowell
  0 siblings, 1 reply; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-03-07 16:36 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
	linux-integrity, linux-kernel

On Fri, Mar 07, 2025 at 10:56:44AM +0000, Jonathan McDowell wrote:
> Auth sessions are lazily flushed since commit df745e25098dc ("tpm:
> Lazily flush the auth session"), so it's expected that we might try to
> start a new session when one is still active.
> 
> Signed-off-by: Jonathan McDowell <noodles@meta.com>
> ---
>  drivers/char/tpm/tpm2-sessions.c | 1 -
>  1 file changed, 1 deletion(-)
> 
> diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> index b70165b588ec..2d2c192ebb14 100644
> --- a/drivers/char/tpm/tpm2-sessions.c
> +++ b/drivers/char/tpm/tpm2-sessions.c
> @@ -982,7 +982,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
>  	int rc;
>  
>  	if (chip->auth) {
> -		dev_warn_once(&chip->dev, "auth session is active\n");
>  		return 0;
>  	}

OK so curly faces should be also removed but I can adjust this
(if you don't mind), so we save all of us from trouble of
going through additional review round?

>  
> -- 
> 2.48.1
> 

BR, Jarkko

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

* Re: [PATCH] tpm: Drop warning when an auth session is active
  2025-03-07 16:36     ` Jarkko Sakkinen
@ 2025-03-07 17:25       ` Jonathan McDowell
  2025-03-07 19:49         ` Jarkko Sakkinen
  0 siblings, 1 reply; 6+ messages in thread
From: Jonathan McDowell @ 2025-03-07 17:25 UTC (permalink / raw)
  To: Jarkko Sakkinen
  Cc: Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
	linux-integrity, linux-kernel

On Fri, Mar 07, 2025 at 06:36:02PM +0200, Jarkko Sakkinen wrote:
> On Fri, Mar 07, 2025 at 10:56:44AM +0000, Jonathan McDowell wrote:
> > Auth sessions are lazily flushed since commit df745e25098dc ("tpm:
> > Lazily flush the auth session"), so it's expected that we might try to
> > start a new session when one is still active.
> > 
> > Signed-off-by: Jonathan McDowell <noodles@meta.com>
> > ---
> >  drivers/char/tpm/tpm2-sessions.c | 1 -
> >  1 file changed, 1 deletion(-)
> > 
> > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> > index b70165b588ec..2d2c192ebb14 100644
> > --- a/drivers/char/tpm/tpm2-sessions.c
> > +++ b/drivers/char/tpm/tpm2-sessions.c
> > @@ -982,7 +982,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
> >  	int rc;
> >  
> >  	if (chip->auth) {
> > -		dev_warn_once(&chip->dev, "auth session is active\n");
> >  		return 0;
> >  	}
> 
> OK so curly faces should be also removed but I can adjust this
> (if you don't mind), so we save all of us from trouble of
> going through additional review round?

Doh! Shoulda caught that. Feel free to do the fix up.

J.

-- 
   "Why? - because it's f***ing    |  .''`.  Debian GNU/Linux Developer
     there!" -- Edmund Hilary      | : :' :  Happy to accept PGP signed
                                   | `. `'   or encrypted mail - RSA
                                   |   `-    key on the keyservers.

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

* Re: [PATCH] tpm: Drop warning when an auth session is active
  2025-03-07 17:25       ` Jonathan McDowell
@ 2025-03-07 19:49         ` Jarkko Sakkinen
  0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2025-03-07 19:49 UTC (permalink / raw)
  To: Jonathan McDowell
  Cc: Peter Huewe, Jason Gunthorpe, James Bottomley, Mimi Zohar,
	linux-integrity, linux-kernel

On Fri, Mar 07, 2025 at 05:25:36PM +0000, Jonathan McDowell wrote:
> On Fri, Mar 07, 2025 at 06:36:02PM +0200, Jarkko Sakkinen wrote:
> > On Fri, Mar 07, 2025 at 10:56:44AM +0000, Jonathan McDowell wrote:
> > > Auth sessions are lazily flushed since commit df745e25098dc ("tpm:
> > > Lazily flush the auth session"), so it's expected that we might try to
> > > start a new session when one is still active.
> > > 
> > > Signed-off-by: Jonathan McDowell <noodles@meta.com>
> > > ---
> > >  drivers/char/tpm/tpm2-sessions.c | 1 -
> > >  1 file changed, 1 deletion(-)
> > > 
> > > diff --git a/drivers/char/tpm/tpm2-sessions.c b/drivers/char/tpm/tpm2-sessions.c
> > > index b70165b588ec..2d2c192ebb14 100644
> > > --- a/drivers/char/tpm/tpm2-sessions.c
> > > +++ b/drivers/char/tpm/tpm2-sessions.c
> > > @@ -982,7 +982,6 @@ int tpm2_start_auth_session(struct tpm_chip *chip)
> > >  	int rc;
> > >  
> > >  	if (chip->auth) {
> > > -		dev_warn_once(&chip->dev, "auth session is active\n");
> > >  		return 0;
> > >  	}
> > 
> > OK so curly faces should be also removed but I can adjust this
> > (if you don't mind), so we save all of us from trouble of
> > going through additional review round?
> 
> Doh! Shoulda caught that. Feel free to do the fix up.

Sure np!

> J.

BR, Jarkko

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

end of thread, other threads:[~2025-03-07 19:49 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <Z8m8G0RfiRyYGH_t@earth.li>
     [not found] ` <Z8oV9lJ4hsHualcP@kernel.org>
2025-03-07 10:56   ` [PATCH] tpm: Drop warning when an auth session is active Jonathan McDowell
2025-03-07 16:36     ` Jarkko Sakkinen
2025-03-07 17:25       ` Jonathan McDowell
2025-03-07 19:49         ` Jarkko Sakkinen
2025-03-07 10:58   ` [PATCH] tpm: Lazily flush auth session when getting random data Jonathan McDowell
2025-03-07 16:34     ` Jarkko Sakkinen

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