[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <D4EOVMA9YR3W.1X1ARMW6APFPP@kernel.org>
Date: Tue, 24 Sep 2024 20:26:47 +0300
From: "Jarkko Sakkinen" <jarkko@...nel.org>
To: "Jarkko Sakkinen" <jarkko@...nel.org>, "James Bottomley"
<James.Bottomley@...senPartnership.com>, <linux-integrity@...r.kernel.org>
Cc: <roberto.sassu@...wei.com>, <mapengyu@...il.com>, "Mimi Zohar"
<zohar@...ux.ibm.com>, "David Howells" <dhowells@...hat.com>, "Paul Moore"
<paul@...l-moore.com>, "James Morris" <jmorris@...ei.org>, "Serge E.
Hallyn" <serge@...lyn.com>, "Peter Huewe" <peterhuewe@....de>, "Jason
Gunthorpe" <jgg@...pe.ca>, <keyrings@...r.kernel.org>,
<linux-security-module@...r.kernel.org>, <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v5 0/5] Lazy flush for the auth session
On Tue Sep 24, 2024 at 7:36 PM EEST, Jarkko Sakkinen wrote:
> On Tue Sep 24, 2024 at 7:33 PM EEST, James Bottomley wrote:
> > On Tue, 2024-09-24 at 19:29 +0300, Jarkko Sakkinen wrote:
> > > On Tue Sep 24, 2024 at 4:48 PM EEST, James Bottomley wrote:
> > [...]
> > > > Patch 3 is completely unnecessary: the null key is only used to
> > > > salt the session and is not required to be resident while the
> > > > session is used (so can be flushed after session creation)
> > > > therefore keeping it around serves no purpose once the session is
> > > > created and simply clutters up the TPM volatile handle slots. (I
> > > > don't know of a case where we use all the slots in a kernel
> > > > operation, but since we don't need it lets not find out when we get
> > > > one). So I advise dropping patch 3.
> > >
> > > Let's go this through just to check I'm understanding.
> > >
> > > Holding null key had radical effect on boot time: it cut it down by
> > > 5 secons down to 15 seconds:
> > >
> > > https://lore.kernel.org/linux-integrity/CALSz7m1WG7fZ9UuO0URgCZEDG7r_wB4Ev_4mOHJThH_d1Ed1nw@mail.gmail.com/
> > >
> > > Then in subsequent version I implemented lazy auth session and boot
> > > time went down to 9.7 seconds.
> > >
> > > So is the point you're trying to make that since auth session is
> > > already held as long as we can and they flushed in synchronous
> > > point too, I can just as well drop patch 3?
> >
> > Yes, because the null key is only used in session generation which is
> > now lazy, it adds or subtracts nothing from the timings. When you're
> > forced to flush the session, the null key goes too, so you again have
> > to restore it from the context. When you can keep the session you
> > don't need the null key because you're not regenerating it.
>
> Yeah, OK, then we're in sync with this. It's evolutionary cruft.
>
> Just had to check that the logic matches how I projected your earlier
> comment because these are sensitive changes.
I'm definitely going keeep 1/5 and 2/5 as they are still bug fixes.
So they will appear in v6 unchanged and perf fixes (which are not
functional fixes) should not be built on top of broken code.
BR, Jarkko
Powered by blists - more mailing lists