Re: [RFC PATCH 0/2] keys: Address lookup_user_key() mutability

[Jarkko Sakkinen <jarkko@kernel.org>]

On Tue, Oct 06, 2026 at 01:52:28PM -0500, Serge Hallyn (AMD) wrote:
> On Mon, Oct 05, 2026 at 09:46:41PM +0300, Jarkko Sakkinen wrote:
> > On Mon, Oct 05, 2026 at 08:09:24AM -0500, Serge E. Hallyn wrote:
> > > On Sun, Oct 04, 2026 at 08:54:06PM +0300, Jarkko Sakkinen wrote:
> > > > On Thu, Sep 24, 2026 at 08:55:16AM +0300, Jarkko Sakkinen wrote:
> > > > > The main objective in these patches is to remove to unneeded mutability
> > > > > from key look ups. It does harm and brings no value so it is pretty obvious
> > > > > to me that addressing this harmful behaviour is what we should do.
> > > > > 
> > > > > I based the first patch on what Jann suggested in [1]. Nothing too clever here
> > > > > and I'm ofc open for further suggestions.
> > > > > 
> > > > > [1] https://lore.kernel.org/keyrings/CAG48ez0XjVSR=M--UBauTm8sJuc+ZbhKzaPa3a6wrSVHyoKevw@mail.gmail.com/
> > > > > 
> > > > > Jarkko Sakkinen (2):
> > > > >   keys: Return user session keyring on lookup
> > > > >   keys: Reject keyring creation with overridden credentials
> > > > > 
> > > > >  Documentation/security/keys/core.rst |  7 ++-
> > > > >  security/keys/process_keys.c         | 65 +++++++++++++++-------------
> > > > >  2 files changed, 40 insertions(+), 32 deletions(-)
> > > > > 
> > > > > --
> > > > > 2.47.3
> > > > > 
> > > > 
> > > > So.. should I move forward to with non-RFC v2?
> > > 
> > > Patch 2 absolutely makes sense, "doc, it hurts when I do this", "don't do
> > > that then."
> > > 
> > > Regarding patch 1, that seems like quite a change in behavior, right?  I
> > > don't see any docs or comments that promise the current behavior, but
> > > has any userspace or subsystem come to depend on it?  I guess that, if so,
> > > then it just has to create a link to the user session keyring like pam
> > > (according to what I've read) does?
> > 
> > Thanks a lot of responding.
> > 
> > I fully agree with you but I'd need help for evaluating things further.
> > 
> > 1/2 based on Jann's email about the topic and my interpretation of the
> > suggestion. I neither used brains nor AI for this and consider the patch
> > merely as a conversation starter (and what a great success it was on doing
> > that) :-)
> 
>  :)
> 
> > All feedback is good feedback at this point. I need supporting code to
> > remember/recall later on so at least this serves that purpose.
> > 
> > Br, Jarkko
> 
> I've been staring at the before and after code for a bit now.
> It does even more than I was thinking :)  But I was trying to
> detail the changed behavior (from userspace's pov), and after
> several attempts, it seems maybe there actually isn't any.
> 
> Q: is there any path whereby the 
> 
> ```
>         else if (test_bit(KEY_FLAG_UID_KEYRING,
>                                     &ctx.cred->session_keyring->flags) &&
>                            lflags & KEY_LOOKUP_CREATE) {
> ```
> 
> case can still happen?  Or was this the only place where we installed
> the user_session keyring as session keyring, so that we can drop this
> branch?

Right, so *I think* that it is not useful and e.g., PAM uses NULL.

At the same time it is harmless. We could remove it but it would be
also uapi change (for granted, useless branch).

I could do it but not without any feedback from David.

Br, Jarkko