* [PATCH v2 1/2] KEYS: Return user session keyring on lookup
@ 2026-10-19 14:56 Jarkko Sakkinen
2026-10-19 14:56 ` [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials Jarkko Sakkinen
2026-10-19 16:16 ` [PATCH v2 1/2] KEYS: Return user session keyring on lookup sashiko-bot
0 siblings, 2 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2026-10-19 14:56 UTC (permalink / raw)
To: David Howells
Cc: keyrings, Eric Biggers, Jarkko Sakkinen, stable, Jann Horn,
Jonathan Corbet, Shuah Khan, Randy Dunlap, Paul Moore,
James Morris, Serge E. Hallyn, open list:DOCUMENTATION, open list,
open list:SECURITY SUBSYSTEM
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.
Cc: stable@vger.kernel.org # v5.10+
Suggested-by: Jann Horn <jannh@google.com>
Closes: https://lore.kernel.org/keyrings/CAG48ez0XjVSR=M--UBauTm8sJuc+ZbhKzaPa3a6wrSVHyoKevw@mail.gmail.com/
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
- Added mandatory tags.
- No code changes.
---
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 1a51a36ce32a..558ffe58b5d6 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.
*/
@@ -664,20 +639,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials
2026-10-19 14:56 [PATCH v2 1/2] KEYS: Return user session keyring on lookup Jarkko Sakkinen
@ 2026-10-19 14:56 ` Jarkko Sakkinen
2026-10-19 15:59 ` David Howells
2026-10-19 16:18 ` sashiko-bot
2026-10-19 16:16 ` [PATCH v2 1/2] KEYS: Return user session keyring on lookup sashiko-bot
1 sibling, 2 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2026-10-19 14:56 UTC (permalink / raw)
To: David Howells
Cc: keyrings, Eric Biggers, Jarkko Sakkinen, stable, Jann Horn,
Jonathan Corbet, Shuah Khan, Randy Dunlap, Paul Moore,
James Morris, Serge E. Hallyn, open list:DOCUMENTATION, open list,
open list:SECURITY SUBSYSTEM
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.
Cc: stable@vger.kernel.org # v5.10+
Suggested-by: Jann Horn <jannh@google.com>
Closes: https://lore.kernel.org/keyrings/CAG48ez0XjVSR=M--UBauTm8sJuc+ZbhKzaPa3a6wrSVHyoKevw@mail.gmail.com/
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
---
v2:
- Added mandatory tags.
- No code changes.
---
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 558ffe58b5d6..ab7f13ccebb1 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.
*/
@@ -577,7 +585,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.
@@ -606,6 +615,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);
@@ -624,6 +638,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);
@@ -645,6 +664,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;
@@ -657,6 +681,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials
2026-10-19 14:56 ` [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials Jarkko Sakkinen
@ 2026-10-19 15:59 ` David Howells
2026-10-19 17:08 ` Jarkko Sakkinen
2026-10-19 16:18 ` sashiko-bot
1 sibling, 1 reply; 6+ messages in thread
From: David Howells @ 2026-10-19 15:59 UTC (permalink / raw)
To: Jarkko Sakkinen
Cc: dhowells, keyrings, Eric Biggers, stable, Jann Horn,
Jonathan Corbet, Shuah Khan, Randy Dunlap, Paul Moore,
James Morris, Serge E. Hallyn, open list:DOCUMENTATION, open list,
open list:SECURITY SUBSYSTEM
Would it make sense to just reject KEY_LOOKUP_CREATE at the top of the
function if the creds are overridden?
David
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials
2026-10-19 15:59 ` David Howells
@ 2026-10-19 17:08 ` Jarkko Sakkinen
0 siblings, 0 replies; 6+ messages in thread
From: Jarkko Sakkinen @ 2026-10-19 17:08 UTC (permalink / raw)
To: David Howells
Cc: keyrings, Eric Biggers, stable, Jann Horn, Jonathan Corbet,
Shuah Khan, Randy Dunlap, Paul Moore, James Morris,
Serge E. Hallyn, open list:DOCUMENTATION, open list,
open list:SECURITY SUBSYSTEM
On Mon, Oct 19, 2026 at 04:59:10PM +0100, David Howells wrote:
> Would it make sense to just reject KEY_LOOKUP_CREATE at the top of the
> function if the creds are overridden?
Let me check..
Yep, I suppose you're right. I'll update the patch.
>
> David
>
Br, Jarkko
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials
2026-10-19 14:56 ` [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials Jarkko Sakkinen
2026-10-19 15:59 ` David Howells
@ 2026-10-19 16:18 ` sashiko-bot
1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-19 16:18 UTC (permalink / raw)
Cc: open list:SECURITY SUBSYSTEM
> 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.
>
> Cc: stable@vger.kernel.org # v5.10+
> Suggested-by: Jann Horn <jannh@google.com>
> Closes: https://lore.kernel.org/keyrings/CAG48ez0XjVSR=M--UBauTm8sJuc+ZbhKzaPa3a6wrSVHyoKevw@mail.gmail.com/
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
Sashiko has reviewed this patch and found no issues.
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019145624.309790-1-jarkko@kernel.org?part=2
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v2 1/2] KEYS: Return user session keyring on lookup
2026-10-19 14:56 [PATCH v2 1/2] KEYS: Return user session keyring on lookup Jarkko Sakkinen
2026-10-19 14:56 ` [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials Jarkko Sakkinen
@ 2026-10-19 16:16 ` sashiko-bot
1 sibling, 0 replies; 6+ messages in thread
From: sashiko-bot @ 2026-10-19 16:16 UTC (permalink / raw)
Cc: open list:SECURITY SUBSYSTEM
Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] keys: NULL dereference in keyctl_session_to_parent() without session keyring
--
commit e79a6a32e7b5e470d5cab49936bcd7b8bfc551d1
Author: Jarkko Sakkinen <jarkko@kernel.org>
KEYS: Return user session keyring on lookup
This commit changes lookup_user_key() so that looking up
KEY_SPEC_SESSION_KEYRING without KEY_LOOKUP_CREATE returns the user session
keyring directly instead of installing it into the current task's
credentials.
> diff --git a/security/keys/process_keys.c b/security/keys/process_keys.c
> index a63c46bb2d148..a5aa056a67259 100644
> --- a/security/keys/process_keys.c
> +++ b/security/keys/process_keys.c
[ ... ]
> @@ -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;
[Severity: Critical]
Could returning user_session here without installing it into the current
task's credentials cause a NULL pointer dereference in
keyctl_session_to_parent()?
In security/keys/keyctl.c, keyctl_session_to_parent() looks up
KEY_SPEC_SESSION_KEYRING without KEY_LOOKUP_CREATE and stores the returned
keyring in cred->session_keyring:
security/keys/keyctl.c:keyctl_session_to_parent() {
keyring_r = lookup_user_key(KEY_SPEC_SESSION_KEYRING, 0, KEY_NEED_LINK);
if (IS_ERR(keyring_r))
return PTR_ERR(keyring_r);
...
cred->session_keyring = key_ref_to_ptr(keyring_r);
...
mycred = current_cred();
pcred = __task_cred(parent);
if (mycred == pcred ||
mycred->session_keyring == pcred->session_keyring) {
ret = 0;
goto unlock;
}
...
/* the keyrings must have the same UID */
if ((pcred->session_keyring &&
!uid_eq(pcred->session_keyring->uid, mycred->euid)) ||
!uid_eq(mycred->session_keyring->uid, mycred->euid))
goto unlock;
}
Previously, lookup_user_key() installed user_session into the current
task's credentials, guaranteeing that mycred->session_keyring was non-NULL.
With this change, if a child process has no session keyring
(mycred->session_keyring is NULL) and its parent has a non-NULL
pcred->session_keyring with matching ownership, the check
mycred->session_keyring == pcred->session_keyring is false, and
!uid_eq(mycred->session_keyring->uid, mycred->euid) dereferences NULL
while holding write_lock_irq(&tasklist_lock) and rcu_read_lock().
Additionally, if both the child and the parent have session_keyring == NULL,
mycred->session_keyring == pcred->session_keyring evaluates to true (NULL ==
NULL), causing keyctl_session_to_parent() to return 0 early without
installing cred->session_keyring onto the parent.
Should keyctl_session_to_parent() check cred->session_keyring instead of
mycred->session_keyring (and avoid the mycred == pcred early exit when
mycred->session_keyring is NULL)?
> } else if (test_bit(KEY_FLAG_UID_KEYRING,
> &ctx.cred->session_keyring->flags) &&
> lflags & KEY_LOOKUP_CREATE) {
--
Sashiko AI review · https://sashiko.dev/#/patchset/20261019145624.309790-1-jarkko@kernel.org?part=1
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-10-19 17:08 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-19 14:56 [PATCH v2 1/2] KEYS: Return user session keyring on lookup Jarkko Sakkinen
2026-10-19 14:56 ` [PATCH v2 2/2] KEYS: Reject keyring creation with overridden credentials Jarkko Sakkinen
2026-10-19 15:59 ` David Howells
2026-10-19 17:08 ` Jarkko Sakkinen
2026-10-19 16:18 ` sashiko-bot
2026-10-19 16:16 ` [PATCH v2 1/2] KEYS: Return user session keyring on lookup sashiko-bot
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.