* [PATCH] keys: Fix key_user use-after-free during ownership changes
@ 2026-08-23 15:28 Chengfeng Ye
0 siblings, 0 replies; 3+ messages in thread
From: Chengfeng Ye @ 2026-08-23 15:28 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, Serge Hallyn
Cc: keyrings, linux-security-module, linux-kernel, Chengfeng Ye
keyctl_chown_key() replaces key->user while holding key->sem and drops
the old key_user reference after releasing the semaphore. The /proc/keys
iterators and find_keyring_by_name() instead dereference key->user while
holding unrelated locks.
This allows the following interleaving:
CPU 0 (/proc/keys) CPU 1 (KEYCTL_CHOWN)
load old key->user
replace key->user
key_user_put(old)
kfree(old)
read old->uid
Serialize the namespace-mapping reads and the pointer replacement with
key_user_lock. This lock already protects final key_user removal, so the
old object cannot be freed while its uid is being read. Keep reading the
quota-owner UID rather than key->uid because those values can legitimately
differ for thread keyrings.
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
security/keys/internal.h | 10 ++++++++++
security/keys/keyctl.c | 2 ++
security/keys/keyring.c | 2 +-
security/keys/proc.c | 4 ++--
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/security/keys/internal.h b/security/keys/internal.h
index b7b622bc36a1..741d547ba5c4 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -70,6 +70,16 @@ extern struct key_user root_key_user;
extern struct key_user *key_user_lookup(kuid_t uid);
extern void key_user_put(struct key_user *user);
+static inline kuid_t key_user_uid(const struct key *key)
+{
+ kuid_t uid;
+
+ spin_lock(&key_user_lock);
+ uid = key->user->uid;
+ spin_unlock(&key_user_lock);
+ return uid;
+}
+
/*
* Key quota limits.
* - root has its own separate limits to everyone else
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index d14ace88e529..c17924609317 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1036,8 +1036,10 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
atomic_inc(&newowner->nikeys);
}
+ spin_lock(&key_user_lock);
zapowner = key->user;
key->user = newowner;
+ spin_unlock(&key_user_lock);
key->uid = uid;
}
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 15bf4af8f282..49f4be934525 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1158,7 +1158,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring)
* grants Search permission and that hasn't been revoked
*/
list_for_each_entry(keyring, &ns->keyring_name_list, name_link) {
- if (!kuid_has_mapping(ns, keyring->user->uid))
+ if (!kuid_has_mapping(ns, key_user_uid(keyring)))
continue;
if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
diff --git a/security/keys/proc.c b/security/keys/proc.c
index 4f4e2c1824f1..8d6d26652aab 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_user_uid(key)))
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, key_user_uid(minkey)))
return minkey;
n = rb_next(&minkey->serial_node);
if (!n)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] keys: Fix key_user use-after-free during ownership changes
@ 2026-08-23 17:04 Chengfeng Ye
2026-08-23 17:08 ` Chengfeng Ye
0 siblings, 1 reply; 3+ messages in thread
From: Chengfeng Ye @ 2026-08-23 17:04 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, Serge Hallyn
Cc: keyrings, linux-security-module, linux-kernel, Chengfeng Ye
keyctl_chown_key() replaces key->user while holding key->sem and drops
the old key_user reference after releasing the semaphore. The /proc/keys
iterators and find_keyring_by_name() instead dereference key->user while
holding unrelated locks.
This allows the following interleaving:
CPU 0 (/proc/keys) CPU 1 (KEYCTL_CHOWN)
load old key->user
replace key->user
key_user_put(old)
kfree(old)
read old->uid
Serialize the namespace-mapping reads and the pointer replacement with
key_user_lock. This lock already protects final key_user removal, so the
old object cannot be freed while its uid is being read. Keep reading the
quota-owner UID rather than key->uid because those values can legitimately
differ for thread keyrings.
Fixes: 454804ab0302 ("keys: make procfiles per-user-namespace")
Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
---
security/keys/internal.h | 10 ++++++++++
security/keys/keyctl.c | 2 ++
security/keys/keyring.c | 2 +-
security/keys/proc.c | 4 ++--
4 files changed, 15 insertions(+), 3 deletions(-)
diff --git a/security/keys/internal.h b/security/keys/internal.h
index b7b622bc36a1..741d547ba5c4 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -70,6 +70,16 @@ extern struct key_user root_key_user;
extern struct key_user *key_user_lookup(kuid_t uid);
extern void key_user_put(struct key_user *user);
+static inline kuid_t key_user_uid(const struct key *key)
+{
+ kuid_t uid;
+
+ spin_lock(&key_user_lock);
+ uid = key->user->uid;
+ spin_unlock(&key_user_lock);
+ return uid;
+}
+
/*
* Key quota limits.
* - root has its own separate limits to everyone else
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index d14ace88e529..c17924609317 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1036,8 +1036,10 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
atomic_inc(&newowner->nikeys);
}
+ spin_lock(&key_user_lock);
zapowner = key->user;
key->user = newowner;
+ spin_unlock(&key_user_lock);
key->uid = uid;
}
diff --git a/security/keys/keyring.c b/security/keys/keyring.c
index 15bf4af8f282..49f4be934525 100644
--- a/security/keys/keyring.c
+++ b/security/keys/keyring.c
@@ -1158,7 +1158,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring)
* grants Search permission and that hasn't been revoked
*/
list_for_each_entry(keyring, &ns->keyring_name_list, name_link) {
- if (!kuid_has_mapping(ns, keyring->user->uid))
+ if (!kuid_has_mapping(ns, key_user_uid(keyring)))
continue;
if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
diff --git a/security/keys/proc.c b/security/keys/proc.c
index 4f4e2c1824f1..8d6d26652aab 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_user_uid(key)))
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, key_user_uid(minkey)))
return minkey;
n = rb_next(&minkey->serial_node);
if (!n)
--
2.43.0
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] keys: Fix key_user use-after-free during ownership changes
2026-08-23 17:04 [PATCH] keys: Fix key_user use-after-free during ownership changes Chengfeng Ye
@ 2026-08-23 17:08 ` Chengfeng Ye
0 siblings, 0 replies; 3+ messages in thread
From: Chengfeng Ye @ 2026-08-23 17:08 UTC (permalink / raw)
To: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn, Serge Hallyn
Cc: keyrings, linux-security-module, linux-kernel
On Mon, Aug 24, 2026 at 1:04 AM Chengfeng Ye <nicoyip.dev@gmail.com> wrote:
>
> keyctl_chown_key() replaces key->user while holding key->sem and drops
> the old key_user reference after releasing the semaphore. The /proc/keys
> iterators and find_keyring_by_name() instead dereference key->user while
> holding unrelated locks.
>
> This allows the following interleaving:
>
> CPU 0 (/proc/keys) CPU 1 (KEYCTL_CHOWN)
> load old key->user
> replace key->user
> key_user_put(old)
> kfree(old)
> read old->uid
>
> Serialize the namespace-mapping reads and the pointer replacement with
> key_user_lock. This lock already protects final key_user removal, so the
> old object cannot be freed while its uid is being read. Keep reading the
> quota-owner UID rather than key->uid because those values can legitimately
> differ for thread keyrings.
>
> Fixes: 454804ab0302 ("keys: make procfiles per-user-namespace")
> Signed-off-by: Chengfeng Ye <nicoyip.dev@gmail.com>
> ---
> security/keys/internal.h | 10 ++++++++++
> security/keys/keyctl.c | 2 ++
> security/keys/keyring.c | 2 +-
> security/keys/proc.c | 4 ++--
> 4 files changed, 15 insertions(+), 3 deletions(-)
>
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index b7b622bc36a1..741d547ba5c4 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -70,6 +70,16 @@ extern struct key_user root_key_user;
> extern struct key_user *key_user_lookup(kuid_t uid);
> extern void key_user_put(struct key_user *user);
>
> +static inline kuid_t key_user_uid(const struct key *key)
> +{
> + kuid_t uid;
> +
> + spin_lock(&key_user_lock);
> + uid = key->user->uid;
> + spin_unlock(&key_user_lock);
> + return uid;
> +}
> +
> /*
> * Key quota limits.
> * - root has its own separate limits to everyone else
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index d14ace88e529..c17924609317 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1036,8 +1036,10 @@ long keyctl_chown_key(key_serial_t id, uid_t user, gid_t group)
> atomic_inc(&newowner->nikeys);
> }
>
> + spin_lock(&key_user_lock);
> zapowner = key->user;
> key->user = newowner;
> + spin_unlock(&key_user_lock);
> key->uid = uid;
> }
>
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 15bf4af8f282..49f4be934525 100644
> --- a/security/keys/keyring.c
> +++ b/security/keys/keyring.c
> @@ -1158,7 +1158,7 @@ struct key *find_keyring_by_name(const char *name, bool uid_keyring)
> * grants Search permission and that hasn't been revoked
> */
> list_for_each_entry(keyring, &ns->keyring_name_list, name_link) {
> - if (!kuid_has_mapping(ns, keyring->user->uid))
> + if (!kuid_has_mapping(ns, key_user_uid(keyring)))
> continue;
>
> if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))
> diff --git a/security/keys/proc.c b/security/keys/proc.c
> index 4f4e2c1824f1..8d6d26652aab 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_user_uid(key)))
> 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, key_user_uid(minkey)))
> return minkey;
> n = rb_next(&minkey->serial_node);
> if (!n)
> --
> 2.43.0
>
Sorry for mistakenly sending this patch twice.
Best,
Chengfeng
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2026-08-23 17:09 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-08-23 17:04 [PATCH] keys: Fix key_user use-after-free during ownership changes Chengfeng Ye
2026-08-23 17:08 ` Chengfeng Ye
-- strict thread matches above, loose matches on Subject: below --
2026-08-23 15:28 Chengfeng Ye
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox