Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths

[Jarkko Sakkinen <jarkko@kernel.org>]

On Mon, Jun 08, 2026 at 08:29:11AM +0300, Jarkko Sakkinen wrote:
> On Mon, Jun 08, 2026 at 06:10:23AM +0300, Jarkko Sakkinen wrote:
> > On Mon, Jun 08, 2026 at 06:06:21AM +0300, Jarkko Sakkinen wrote:
> > > On Tue, May 26, 2026 at 10:48:38AM +0800, Shaomin Chen wrote:
> > > > keyctl_instantiate_key_common() reads request_key_auth from the assumed
> > > > auth key before copying an instantiation payload from userspace.  The copy
> > > > can fault and sleep.  If the request completes and revokes the auth key in
> > > > that window, the auth payload can be detached and freed before the
> > > > instantiate path uses it again.
> > > > 
> > > > A request-key helper reproducer can trigger this race.  One helper child
> > > > blocks in KEYCTL_INSTANTIATE_IOV while the original helper instantiates the
> > > > requested key and returns.  KASAN then reports a use-after-free from the
> > > > stale request_key_auth payload in keyctl_instantiate_key_common().
> > > > 
> > > > Give request_key_auth payloads a refcount.  Take a payload reference while
> > > 
> > > Please, name concrete things accurately. I.e. 'usage' in this case. If
> > > you have a name, use it instead of obfuscating generalizations.
> > > 
> > > > authkey->sem stabilizes the payload and revocation state.  Hold that
> > > > reference across the instantiate and reject paths.  Drop the auth key
> > > > owning reference from revoke and destroy.
> > > > 
> > > > Reported-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > > Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@gmail.com
> > > > Signed-off-by: Shaomin Chen <eeesssooo020@gmail.com>
> > > > ---
> > > >  include/keys/request_key_auth-type.h |  2 ++
> > > >  security/keys/internal.h             |  2 ++
> > > >  security/keys/keyctl.c               | 24 +++++++++++++++-----
> > > >  security/keys/request_key_auth.c     | 33 ++++++++++++++++++++++++++--
> > > >  4 files changed, 53 insertions(+), 8 deletions(-)
> > > 
> > > So first, couple of things.
> > > 
> > > I'm not going to test not that well documented involving OOT driver.
> > 
> > Oops, sorry typo. "not that well documented reproducer" :-)
> > 
> > But it is cool we just then need to draw the picture.
> 
> I think I got this:
> 
> A: request_key()       B: KEYCTL_INSTANTIATE_IOV
> ----------------       -------------------------
> create auth key
> store rka in auth key
> wait for helper
>                        get auth key
>                        load rka from auth key
>                        copy user payload
>                        sleep on #PF
> helper completed
> detach and free rka
> destroy auth key
>                        wake up
>                        use rka->target_key
>                        **USE-AFTER-FREE**
> 
> So nothing really complicated here, is there?

Send v2 with the code changes that I proposed as we want to the change
as ergonomic as possible.

Use this as the commit message:

    keys: Pin request_key_auth payload in instantiate paths

    A: request_key()       B: KEYCTL_INSTANTIATE_IOV
    ----------------       -------------------------
    create auth key
    store rka in auth key
    wait for helper
                           get auth key
                           load rka from auth key
                           copy user payload
                           sleep on #PF

    helper completed
    detach and free rka
    destroy auth key
                           wake up
                           use rka->target_key
                           **USE-AFTER-FREE**

    Give request_key_auth payloads a refcount.  Take a payload reference while
    authkey->sem stabilizes the payload and revocation state.  Hold that
    reference across the instantiate and reject paths.  Drop the auth key
    owning reference from revoke and destroy.

    [jarkko: Replaced the first two paragraphs of text with a concurrency scenario.]

And it includes also the remark at the end.

BR, Jarkko