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

[eee sss]

Thanks, Jarkko. Sorry for the delayed response.

I checked the commit in for-next-keys. The updated commit message and the
cleanup look good to me.

Best,
Shaomin

On Wed, 10 Jun 2026 13:16:01 +0300, Jarkko Sakkinen <jarkko@xxxxxxxxxx> wrote:
> On Mon, Jun 08, 2026 at 08:42:21AM +0300, Jarkko Sakkinen wrote:
> > 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@xxxxxxxxx>
> > > > > > Closes: https://lore.kernel.org/r/20260519144403.436694-1-eeesssooo020@xxxxxxxxx
> > > > > > Signed-off-by: Shaomin Chen <eeesssooo020@xxxxxxxxx>
> > > > > > ---
> > > > > > 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
>
> Nothing heard so I pushed:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/jarkko/linux-tpmdd.git/commit/?h=for-next-keys&id=9feb0bb3468e863b2b82a2eabfaeec4c7c44b90c
>
> It has the commit message change.
>
> BR, Jarkko