mirror of
https://github.com/torvalds/linux.git
synced 2026-11-07 06:17:02 +01:00
keys: Avoid owner account dereferences in /proc/keys
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: 454804ab03 ("keys: make procfiles per-user-namespace")
Assisted-by: LLM
Suggested-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
Reviewed-by: Jarkko Sakkinen <jarkko@kernel.org>
Link: https://lore.kernel.org/r/pm-keys-proc-owner-uid-followup-20261017-v2-8f8f34a41eb1bbb94ef0@gmail.com
Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
This commit is contained in:
parent
de891c73c1
commit
30a9dac7c6
|
|
@ -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)
|
||||
|
|
|
|||
Loading…
Reference in New Issue
Block a user