[PATCH] keys: Avoid owner account dereferences in /proc/keys

[PATCH] keys: Avoid owner account dereferences in /proc/keys

[Cen Zhang]

The namespace filters in find_ge_key() and key_serial_next() read
key->user->uid while traversing the serial tree under key_serial_lock.
That lock keeps the key allocated, but it does not protect the separate
key_user from ownership changes.

A /proc/keys read reaches find_ge_key() through proc_keys_start(), and
key_serial_next() through proc_keys_next(). The lock stays held until
proc_keys_stop().

A successful privileged KEYCTL_CHOWN on the same key replaces key->user
under key->sem and drops the old owner's reference after releasing the
semaphore. If that was the old dynamic key_user's last reference, a
/proc/keys reader can access freed memory:

    /proc/keys reader                   KEYCTL_CHOWN
    proc_keys_start()                    keyctl_chown_key()
      spin_lock(key_serial_lock)          down_write(key->sem)
      find_ge_key()
        old = minkey->user
                                          zapowner = key->user
                                          key->user = newowner
                                          key->uid = uid
                                          up_write(key->sem)
                                          key_put(key)
                                          key_user_put(zapowner)
                                            kfree(zapowner)
        read old->uid -> use-after-free
    proc_keys_stop()
      spin_unlock(key_serial_lock)

Use the inline key->uid in both filters, following the approach already
used by find_keyring_by_name(). The serial-tree lock keeps the containing
key alive, so filtering no longer depends on the quota owner's lifetime.
This also uses the same owner UID as proc_keys_show() and the permission
check.

For thread keyrings, key_fsuid_changed() can update key->uid without
changing the quota owner. The filters now follow that current owner UID,
including when its namespace mapping differs from the quota owner's.

Fixes: 454804ab0302 ("keys: make procfiles per-user-namespace")
Suggested-by: Eric Biggers <ebiggers@kernel.org>
Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

---
 security/keys/proc.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/security/keys/proc.c b/security/keys/proc.c
index 4f4e2c1824f1..aea06a74d0fe 100644
--- a/security/keys/proc.c
+++ b/security/keys/proc.c
@@ -68,7 +68,7 @@ static struct rb_node *key_serial_next(struct seq_file *p, struct rb_node *n)
 	n = rb_next(n);
 	while (n) {
 		struct key *key = rb_entry(n, struct key, serial_node);
-		if (kuid_has_mapping(user_ns, key->user->uid))
+		if (kuid_has_mapping(user_ns, key->uid))
 			break;
 		n = rb_next(n);
 	}
@@ -100,7 +100,7 @@ static struct key *find_ge_key(struct seq_file *p, key_serial_t id)
 		return NULL;
 
 	for (;;) {
-		if (kuid_has_mapping(user_ns, minkey->user->uid))
+		if (kuid_has_mapping(user_ns, minkey->uid))
 			return minkey;
 		n = rb_next(&minkey->serial_node);
 		if (!n)

base-commit: a626e44fd64c456d373ae8c28b939197b9446a02

Re: [PATCH] keys: Avoid owner account dereferences in /proc/keys

[sashiko-bot]

> The namespace filters in find_ge_key() and key_serial_next() read
> key->user->uid while traversing the serial tree under key_serial_lock.
> That lock keeps the key allocated, but it does not protect the separate
> key_user from ownership changes.
> 
> A /proc/keys read reaches find_ge_key() through proc_keys_start(), and
> key_serial_next() through proc_keys_next(). The lock stays held until
> proc_keys_stop().
> 
> A successful privileged KEYCTL_CHOWN on the same key replaces key->user
> under key->sem and drops the old owner's reference after releasing the
> semaphore. If that was the old dynamic key_user's last reference, a
> /proc/keys reader can access freed memory:
> 
>     /proc/keys reader                   KEYCTL_CHOWN
> [ ... ]
> 
> Fixes: 454804ab0302 ("keys: make procfiles per-user-namespace")
> Suggested-by: Eric Biggers <ebiggers@kernel.org>
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

Sashiko has reviewed this patch and found no issues.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/pm-keys-proc-owner-uid-followup-20261017-v2-8f8f34a41eb1bbb94ef0@gmail.com?part=1

Re: [PATCH] keys: Avoid owner account dereferences in /proc/keys

[Jarkko Sakkinen]

On Sat, Oct 17, 2026 at 04:16:22PM +0800, Cen Zhang wrote:
> The namespace filters in find_ge_key() and key_serial_next() read
> key->user->uid while traversing the serial tree under key_serial_lock.
> That lock keeps the key allocated, but it does not protect the separate
> key_user from ownership changes.
> 
> A /proc/keys read reaches find_ge_key() through proc_keys_start(), and
> key_serial_next() through proc_keys_next(). The lock stays held until
> proc_keys_stop().
> 
> A successful privileged KEYCTL_CHOWN on the same key replaces key->user
> under key->sem and drops the old owner's reference after releasing the
> semaphore. If that was the old dynamic key_user's last reference, a
> /proc/keys reader can access freed memory:
> 
>     /proc/keys reader                   KEYCTL_CHOWN
>     proc_keys_start()                    keyctl_chown_key()
>       spin_lock(key_serial_lock)          down_write(key->sem)
>       find_ge_key()
>         old = minkey->user
>                                           zapowner = key->user
>                                           key->user = newowner
>                                           key->uid = uid
>                                           up_write(key->sem)
>                                           key_put(key)
>                                           key_user_put(zapowner)
>                                             kfree(zapowner)
>         read old->uid -> use-after-free
>     proc_keys_stop()
>       spin_unlock(key_serial_lock)
> 
> Use the inline key->uid in both filters, following the approach already
> used by find_keyring_by_name(). The serial-tree lock keeps the containing
> key alive, so filtering no longer depends on the quota owner's lifetime.
> This also uses the same owner UID as proc_keys_show() and the permission
> check.
> 
> For thread keyrings, key_fsuid_changed() can update key->uid without
> changing the quota owner. The filters now follow that current owner UID,
> including when its namespace mapping differs from the quota owner's.
> 
> Fixes: 454804ab0302 ("keys: make procfiles per-user-namespace")
> Suggested-by: Eric Biggers <ebiggers@kernel.org>
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>

These tags could be ordered correctly.

> 
> ---
>  security/keys/proc.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/security/keys/proc.c b/security/keys/proc.c
> index 4f4e2c1824f1..aea06a74d0fe 100644
> --- a/security/keys/proc.c
> +++ b/security/keys/proc.c
> @@ -68,7 +68,7 @@ static struct rb_node *key_serial_next(struct seq_file *p, struct rb_node *n)
>  	n = rb_next(n);
>  	while (n) {
>  		struct key *key = rb_entry(n, struct key, serial_node);
> -		if (kuid_has_mapping(user_ns, key->user->uid))
> +		if (kuid_has_mapping(user_ns, key->uid))
>  			break;
>  		n = rb_next(n);
>  	}
> @@ -100,7 +100,7 @@ static struct key *find_ge_key(struct seq_file *p, key_serial_t id)
>  		return NULL;
>  
>  	for (;;) {
> -		if (kuid_has_mapping(user_ns, minkey->user->uid))
> +		if (kuid_has_mapping(user_ns, minkey->uid))
>  			return minkey;
>  		n = rb_next(&minkey->serial_node);
>  		if (!n)
> 
> base-commit: a626e44fd64c456d373ae8c28b939197b9446a02

I don't want to circulate this further.


Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>

Br, Jarkko