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

[Jarkko Sakkinen]

On Fri, Oct 16, 2026 at 08:51:56PM -0700, Eric Biggers wrote:
> 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@xxxxxxxxx>
> 
> Don't key_serial_next() and find_ge_key() (used when reading from
> /proc/keys) have the same bug?

With just a quick Saturday skim, yep, I think so.

> 
> - Eric

Br, Jarkko