Re: [PATCH v2] keys: Avoid the owner account dereference in named keyring lookup

[Jarkko Sakkinen <jarkko@kernel.org>]

On Fri, Oct 09, 2026 at 01:11:59PM +0800, Cen Zhang wrote:
> Named keyring lookup must keep the storage containing the owner UID
> alive through the namespace mapping check.  find_keyring_by_name()
> reads keyring->user->uid under keyring_name_lock, but that lock protects
> the keyring's name entry and allocation, not its separate key_user.
> 
> A named session-keyring join can overlap a privileged KEYCTL_CHOWN on
> another CPU.  If the keyring holds the last reference to its old dynamic
> key_user and the ownership transfer succeeds, the following ordering is
> possible:
> 
>     Named join                         Chown
>     find_keyring_by_name()              keyctl_chown_key()
>       read_lock(keyring_name_lock)       down_write(key->sem)
>       load old keyring->user
>                                          replace key->user and key->uid
>                                          up_write(key->sem)
>                                          key_put(key)
>                                          key_user_put(old user): free
>       read old user->uid
>       read_unlock(keyring_name_lock)
> 
> Chown neither takes keyring_name_lock nor key_session_mutex, so it can
> free the old account between the pointer load and the UID read.  The
> lookup then reads freed memory even though the keyring itself is alive.
> 
> Use keyring->uid for the mapping check.  key_alloc() initializes this
> inline owner UID and keyctl_chown_key() updates it on chown.
> The existing name lock keeps its containing keyring allocated throughout
> the check, so lookup no longer depends on the account's lifetime.  This
> also matches the owner UID used by the permission check.
> 
> KASAN report as below:
> 
>     BUG: KASAN: slab-use-after-free in find_keyring_by_name+0x577/0x5d0
>     Read of size 4 at addr ffff8881128ae2ec by task keycase/500
> 
> Fixes: 2ea190d0a006 ("keys: skip keys from another user namespace")
> Assisted-by: LLM
> Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
> ---
> Changes in v2:
> - Trim the KASAN report to its two-line fault summary.
> 
> Link to v1: https://lore.kernel.org/r/pm-key-management-objects-candidate-0002-v2-dd6c20a1d92928b33ab7@gmail.com
> 
> diff --git a/security/keys/keyring.c b/security/keys/keyring.c
> index 15bf4af8f28218ec3f12c97630d1c76939af7eca..46f774be72967a9bd16b0ddfdf323eda36edeff4 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, keyring->uid))
>  			continue;
>  
>  		if (test_bit(KEY_FLAG_REVOKED, &keyring->flags))

Thanks.


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

Br, Jarkko