Re: [PATCH v5 0/5] Lazy flush for the auth session

[Jarkko Sakkinen]

On Tue, 2024-10-01 at 14:10 -0400, Mimi Zohar wrote:
> On Sun, 2024-09-22 at 20:51 +0300, Jarkko Sakkinen wrote:
> > On Sat Sep 21, 2024 at 3:08 PM EEST, Jarkko Sakkinen wrote:
> > > This patch set aims to fix:
> > > https://bugzilla.kernel.org/show_bug.cgi?id=219229.
> > > 
> > > The baseline for the series is the v6.11 tag.
> > > 
> > > v4:
> > > https://lore.kernel.org/linux-integrity/20240918203559.192605-1-jarkko@xxxxxxxxxx/
> > > v3:
> > > https://lore.kernel.org/linux-integrity/20240917154444.702370-1-jarkko@xxxxxxxxxx/
> > > v2:
> > > https://lore.kernel.org/linux-integrity/20240916110714.1396407-1-jarkko@xxxxxxxxxx/
> > > v1:
> > > https://lore.kernel.org/linux-integrity/20240915180448.2030115-1-jarkko@xxxxxxxxxx/
> > > 
> > > Jarkko Sakkinen (5):
> > >   tpm: Return on tpm2_create_null_primary() failure
> > >   tpm: Implement tpm2_load_null() rollback
> > >   tpm: flush the null key only when /dev/tpm0 is accessed
> > >   tpm: Allocate chip->auth in tpm2_start_auth_session()
> > >   tpm: flush the auth session only when /dev/tpm0 is open
> > > 
> > >  drivers/char/tpm/tpm-chip.c       |  14 ++++
> > >  drivers/char/tpm/tpm-dev-common.c |   8 +++
> > >  drivers/char/tpm/tpm-interface.c  |  10 ++-
> > >  drivers/char/tpm/tpm2-cmd.c       |   3 +
> > >  drivers/char/tpm/tpm2-sessions.c  | 109 ++++++++++++++++++------
> > > ------
> > >  include/linux/tpm.h               |   2 +
> > >  6 files changed, 102 insertions(+), 44 deletions(-)
> > 
> > 
> > Roberto, James, speaking of digest cache. This patch set has no aim
> > to
> > fix those issues but I do believe that it should improve also that 
> > feature.
> > 
> > If I don't get soon patch reviews for the patch set, I'll pick the
> > 2nd
> > best option: disable bus encryption on all architectures including
> > x86
> > and ARM64 (being by default on).
> > 
> > It's a force majeure situation. I know this would sort out the
> > issue
> > but I really cannot send these as a pull request with zero reviewe-
> > by's.
> > 
> > I expect this to be closed by tomorrow.
> 
> Jarkko, sorry to be so late to this discussion.  The bus
> HMAC/encryption really
> impacts IMA as well.  Even with this patch set, it's slow.  My
> preference would
> be to disable bus encryption on all architectures until there is a
> boot/runtime
> option allowing it to be disabled for IMA as discussed in the other
> thread.

No worries, I was getting nervous because of a job switch, now
I have time since cannot move this forward for week or two anyway
:-)

I'm totally +1 to make bus encyption opt-in instead of opt-out.
It's just not there yet.

My fixes fix one use case, i.e. the boot process for AMD, so in
that sense they are totally legit. But it is pretty clear by now
that tons of similar patches and small tweaks will be required.

As it is my 2nd work week, I can implement such patch, *next
week*. Up until that there is time to give any feedback.

> 
> In the other thread, I also mentioned that the Kconfig is incorrectly
> worded. 
> The performance degradation is not limited to encryption, but the
> HMAC itself. 
> Please change "Saying Y here adds some encryption overhead to all
> kernel to TPM
> transactions." to "Saying Y here adds overhead to all kernel to TPM
> transactions."

I'll keep this in mind!

I'd prefer to do a single patch set probably with my previous fixes
and this, as they are already tested by the reporter anyway and pile
this as a new patch on top. I.e. have basically all that I'd put to
the next PR.

> 
> thanks,
> 
> Mimi

BR, Jarkko