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

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

[Jarkko Sakkinen]

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

[RFC PATCH 1/2] keys: Return user session keyring on lookup

[Jarkko Sakkinen]

A process without a session keyring looking up KEY_SPEC_SESSION_KEYRING
without KEY_LOOKUP_CREATE mutates the credentials. This causes struct creds
instances shared with other subsystems to become stale.

Address this by returning the resolved user session keyring directly
without installing it into credentials.

Suggested-by: Jann Horn <jannh@google.com>
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 Documentation/security/keys/core.rst |  6 ++--
 security/keys/process_keys.c         | 44 +++++++---------------------
 2 files changed, 14 insertions(+), 36 deletions(-)

diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index 326b8a973828..c81a3a9fe236 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -165,8 +165,10 @@ The key service provides a number of features besides keys:
      When a process changes its real UID, if it used to have no session key, it
      will be subscribed to the default session key for the new UID.
 
-     If a process attempts to access its session key when it doesn't have one,
-     it will be subscribed to the default for its current UID.
+     If a process attempts to access its session keyring when it doesn't have
+     one, the default user session keyring for its current UID is returned
+     without being installed into its credentials.  If creation is requested,
+     an anonymous session keyring is installed.
 
   *  Each user has two quotas against which the keys they own are tracked. One
      limits the total number of keys and keyrings, the other limits the total
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index a63c46bb2d14..a5aa056a6725 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -346,31 +346,6 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
 	return 0;
 }
 
-/*
- * Install the given keyring as the session keyring of the current task,
- * replacing the existing one if any.  If the given keyring is NULL, then
- * install a new anonymous session keyring.
- *
- * Return: 0 on success; -errno on failure.
- */
-static int install_session_keyring(struct key *keyring)
-{
-	struct cred *new;
-	int ret;
-
-	new = prepare_creds();
-	if (!new)
-		return -ENOMEM;
-
-	ret = install_session_keyring_to_cred(new, keyring);
-	if (ret < 0) {
-		abort_creds(new);
-		return ret;
-	}
-
-	return commit_creds(new);
-}
-
 /*
  * Handle the fsuid changing.
  */
@@ -665,20 +640,21 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 
 	case KEY_SPEC_SESSION_KEYRING:
 		if (!ctx.cred->session_keyring) {
-			/* always install a session keyring upon access if one
-			 * doesn't exist yet */
 			ret = look_up_user_keyrings(NULL, &user_session);
 			if (ret < 0)
 				goto error;
-			if (lflags & KEY_LOOKUP_CREATE)
+
+			if (lflags & KEY_LOOKUP_CREATE) {
+				key_put(user_session);
 				ret = join_session_keyring(NULL);
-			else
-				ret = install_session_keyring(user_session);
+				if (ret < 0)
+					goto error;
+				goto reget_creds;
+			}
 
-			key_put(user_session);
-			if (ret < 0)
-				goto error;
-			goto reget_creds;
+			key = user_session;
+			key_ref = make_key_ref(key, 1);
+			break;
 		} else if (test_bit(KEY_FLAG_UID_KEYRING,
 				    &ctx.cred->session_keyring->flags) &&
 			   lflags & KEY_LOOKUP_CREATE) {
-- 
2.47.3

[RFC PATCH 2/2] keys: Reject keyring creation with overridden credentials

[Jarkko Sakkinen]

If the current task is running with overridden credentials calling
commit_creds() is forbidden and triggers a BUG_ON().

Reject keyring creation with -EPERM when credentials are overridden.

Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
 Documentation/security/keys/core.rst |  3 ++-
 security/keys/process_keys.c         | 31 +++++++++++++++++++++++++++-
 2 files changed, 32 insertions(+), 2 deletions(-)

diff --git a/Documentation/security/keys/core.rst b/Documentation/security/keys/core.rst
index c81a3a9fe236..9779ce71df47 100644
--- a/Documentation/security/keys/core.rst
+++ b/Documentation/security/keys/core.rst
@@ -168,7 +168,8 @@ The key service provides a number of features besides keys:
      If a process attempts to access its session keyring when it doesn't have
      one, the default user session keyring for its current UID is returned
      without being installed into its credentials.  If creation is requested,
-     an anonymous session keyring is installed.
+     an anonymous session keyring is installed, failing with -EPERM if
+     credentials are overridden.
 
   *  Each user has two quotas against which the keys they own are tracked. One
      limits the total number of keys and keyrings, the other limits the total
diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
index a5aa056a6725..bd68dbc028e2 100644
--- a/security/keys/process_keys.c
+++ b/security/keys/process_keys.c
@@ -346,6 +346,14 @@ int install_session_keyring_to_cred(struct cred *cred, struct key *keyring)
 	return 0;
 }
 
+/*
+ * Determine whether the current task is running with overridden credentials.
+ */
+static bool cred_overridden(void)
+{
+	return current_cred() != current_real_cred();
+}
+
 /*
  * Handle the fsuid changing.
  */
@@ -578,7 +586,8 @@ bool lookup_user_key_possessed(const struct key *key,
  * to a key or the best found key was a negative key; -EKEYREVOKED or
  * -EKEYEXPIRED if the best found key was revoked or expired; -EACCES if the
  * found key doesn't grant the requested permit or the LSM denied access to it;
- * or -ENOMEM if a special keyring couldn't be created.
+ * -ENOMEM if a special keyring couldn't be created; or -EPERM if creating one
+ * while credentials are overridden.
  *
  * In the case of a successful return, the possession attribute is set on the
  * returned key reference.
@@ -607,6 +616,11 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 			if (!(lflags & KEY_LOOKUP_CREATE))
 				goto error;
 
+			if (cred_overridden()) {
+				key_ref = ERR_PTR(-EPERM);
+				goto error;
+			}
+
 			ret = install_thread_keyring();
 			if (ret < 0) {
 				key_ref = ERR_PTR(ret);
@@ -625,6 +639,11 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 			if (!(lflags & KEY_LOOKUP_CREATE))
 				goto error;
 
+			if (cred_overridden()) {
+				key_ref = ERR_PTR(-EPERM);
+				goto error;
+			}
+
 			ret = install_process_keyring();
 			if (ret < 0) {
 				key_ref = ERR_PTR(ret);
@@ -646,6 +665,11 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 
 			if (lflags & KEY_LOOKUP_CREATE) {
 				key_put(user_session);
+				if (cred_overridden()) {
+					key_ref = ERR_PTR(-EPERM);
+					goto error;
+				}
+
 				ret = join_session_keyring(NULL);
 				if (ret < 0)
 					goto error;
@@ -658,6 +682,11 @@ key_ref_t lookup_user_key(key_serial_t id, unsigned long lflags,
 		} else if (test_bit(KEY_FLAG_UID_KEYRING,
 				    &ctx.cred->session_keyring->flags) &&
 			   lflags & KEY_LOOKUP_CREATE) {
+			if (cred_overridden()) {
+				key_ref = ERR_PTR(-EPERM);
+				goto error;
+			}
+
 			ret = join_session_keyring(NULL);
 			if (ret < 0)
 				goto error;
-- 
2.47.3

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

[Jarkko Sakkinen]

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?

Note that some tags (e.g., Fixes) are missing simply because this is RFC.

Br, Jarkko

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

[Serge E. Hallyn]

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?

> Note that some tags (e.g., Fixes) are missing simply because this is RFC.
> 
> Br, Jarkko

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

[Jarkko Sakkinen]

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

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

[Serge Hallyn (AMD)]

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?

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

[Jarkko Sakkinen]

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

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

[Serge E. Hallyn]

On Thu, Oct 08, 2026 at 05:26:46PM +0300, Jarkko Sakkinen wrote:
> 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).

There's

https://sources.debian.org/src/e2fsprogs/1.47.4-1/misc/e4crypt.c?hl=568#L568

which bemoans that:
	 * The fact that the keyctl system call and the add_key system
	 * call treats KEY_SPEC_SESSION_KEYRING differently when a
	 * session keyring does not exist is very unfortunate and
	 * confusing, but so it goes...

Near as I can tell, add_key just always uses KEY_LOOKUP_CREATE?

And https://sources.debian.org/src/krb5/1.22.1-3/src/lib/krb5/ccache/cc_keyring.c?hl=307#L307

I'm not sure whether previous kernels did that, or whether the authors
mixed up testing with and without KEY_LOOKUP_CREATE.

> I could do it but not without any feedback from David.
> 
> Br, Jarkko

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

[Jarkko Sakkinen]

On Thu, Oct 08, 2026 at 10:25:48AM -0500, Serge E. Hallyn wrote:
> On Thu, Oct 08, 2026 at 05:26:46PM +0300, Jarkko Sakkinen wrote:
> > 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).
> 
> There's
> 
> https://sources.debian.org/src/e2fsprogs/1.47.4-1/misc/e4crypt.c?hl=568#L568
> 
> which bemoans that:
> 	 * The fact that the keyctl system call and the add_key system
> 	 * call treats KEY_SPEC_SESSION_KEYRING differently when a
> 	 * session keyring does not exist is very unfortunate and
> 	 * confusing, but so it goes...
> 
> Near as I can tell, add_key just always uses KEY_LOOKUP_CREATE?
> 
> And https://sources.debian.org/src/krb5/1.22.1-3/src/lib/krb5/ccache/cc_keyring.c?hl=307#L307
> 
> I'm not sure whether previous kernels did that, or whether the authors
> mixed up testing with and without KEY_LOOKUP_CREATE.

Ugh, I feel that I always to go the code from beginning to check these
up but those workarounds are still valid i.e., these patches do not
address write divergence.

I would summarize my fixes along the lines of:

1. 1/2 exactly removes cred creation as a side-effect invisible to the
   caller..
2. 2/2 handles request_key()'s from nfsd and other in-kernel daemons.
   I.e. it transforms BUG_ON() into -EPERM.

I'd like to keep this patch set strictly within scope and somewhat
surgical if/when it is applied.

And it looks to me a lot that this might well be the smallest possible
change that addresses exactly unwanted BUG_ON() conditions pointed out
by Jann surpsingly well.

Br, Jarkko