* [PATCH] keys: Pin request_key_auth payload in instantiate paths
@ 2026-05-26 2:48 Shaomin Chen
2026-05-29 22:53 ` Jarkko Sakkinen
0 siblings, 1 reply; 2+ messages in thread
From: Shaomin Chen @ 2026-05-26 2:48 UTC (permalink / raw)
To: keyrings, linux-security-module, linux-kernel
Cc: David Howells, Jarkko Sakkinen, Paul Moore, James Morris,
Serge E. Hallyn
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
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(-)
diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
index 36b89a933310..01e42ee5f409 100644
--- a/include/keys/request_key_auth-type.h
+++ b/include/keys/request_key_auth-type.h
@@ -9,12 +9,14 @@
#define _KEYS_REQUEST_KEY_AUTH_TYPE_H
#include <linux/key.h>
+#include <linux/refcount.h>
/*
* Authorisation record for request_key().
*/
struct request_key_auth {
struct rcu_head rcu;
+ refcount_t usage;
struct key *target_key;
struct key *dest_keyring;
const struct cred *cred;
diff --git a/security/keys/internal.h b/security/keys/internal.h
index 2cffa6dc8255..b7b622bc36a1 100644
--- a/security/keys/internal.h
+++ b/security/keys/internal.h
@@ -208,6 +208,8 @@ extern struct key *request_key_auth_new(struct key *target,
const void *callout_info,
size_t callout_len,
struct key *dest_keyring);
+struct request_key_auth *request_key_auth_get(struct key *authkey);
+void request_key_auth_put(struct request_key_auth *rka);
extern struct key *key_get_instantiation_authkey(key_serial_t target_id);
diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
index ef855d69c97a..d14ace88e529 100644
--- a/security/keys/keyctl.c
+++ b/security/keys/keyctl.c
@@ -1197,9 +1197,13 @@ static long keyctl_instantiate_key_common(key_serial_t id,
if (!instkey)
goto error;
- rka = instkey->payload.data[0];
- if (rka->target_key->serial != id)
+ rka = request_key_auth_get(instkey);
+ if (!rka) {
+ ret = -EKEYREVOKED;
goto error;
+ }
+ if (rka->target_key->serial != id)
+ goto error_put_rka;
/* pull the payload in if one was supplied */
payload = NULL;
@@ -1208,7 +1212,7 @@ static long keyctl_instantiate_key_common(key_serial_t id,
ret = -ENOMEM;
payload = kvmalloc(plen, GFP_KERNEL);
if (!payload)
- goto error;
+ goto error_put_rka;
ret = -EFAULT;
if (!copy_from_iter_full(payload, plen, from))
@@ -1234,6 +1238,8 @@ static long keyctl_instantiate_key_common(key_serial_t id,
error2:
kvfree_sensitive(payload, plen);
+error_put_rka:
+ request_key_auth_put(rka);
error:
return ret;
}
@@ -1358,15 +1364,19 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
if (!instkey)
goto error;
- rka = instkey->payload.data[0];
- if (rka->target_key->serial != id)
+ rka = request_key_auth_get(instkey);
+ if (!rka) {
+ ret = -EKEYREVOKED;
goto error;
+ }
+ if (rka->target_key->serial != id)
+ goto error_put_rka;
/* find the destination keyring if present (which must also be
* writable) */
ret = get_instantiation_keyring(ringid, rka, &dest_keyring);
if (ret < 0)
- goto error;
+ goto error_put_rka;
/* instantiate the key and link it into a keyring */
ret = key_reject_and_link(rka->target_key, timeout, error,
@@ -1379,6 +1389,8 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
if (ret == 0)
keyctl_change_reqkey_auth(NULL);
+error_put_rka:
+ request_key_auth_put(rka);
error:
return ret;
}
diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
index a7d7538c1f70..282e09d8fa46 100644
--- a/security/keys/request_key_auth.c
+++ b/security/keys/request_key_auth.c
@@ -23,6 +23,7 @@ static void request_key_auth_describe(const struct key *, struct seq_file *);
static void request_key_auth_revoke(struct key *);
static void request_key_auth_destroy(struct key *);
static long request_key_auth_read(const struct key *, char *, size_t);
+static void request_key_auth_rcu_disposal(struct rcu_head *);
/*
* The request-key authorisation key type definition.
@@ -115,6 +116,31 @@ static void free_request_key_auth(struct request_key_auth *rka)
kfree(rka);
}
+/*
+ * Take a reference to the request-key authorisation payload so callers can
+ * drop authkey->sem before doing operations that may sleep.
+ */
+struct request_key_auth *request_key_auth_get(struct key *authkey)
+{
+ struct request_key_auth *rka;
+
+ down_read(&authkey->sem);
+ rka = dereference_key_locked(authkey);
+ if (rka && !test_bit(KEY_FLAG_REVOKED, &authkey->flags))
+ refcount_inc(&rka->usage);
+ else
+ rka = NULL;
+ up_read(&authkey->sem);
+
+ return rka;
+}
+
+void request_key_auth_put(struct request_key_auth *rka)
+{
+ if (rka && refcount_dec_and_test(&rka->usage))
+ call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
+}
+
/*
* Dispose of the request_key_auth record under RCU conditions
*/
@@ -136,8 +162,10 @@ static void request_key_auth_revoke(struct key *key)
struct request_key_auth *rka = dereference_key_locked(key);
kenter("{%d}", key->serial);
+ if (!rka)
+ return;
rcu_assign_keypointer(key, NULL);
- call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
+ request_key_auth_put(rka);
}
/*
@@ -150,7 +178,7 @@ static void request_key_auth_destroy(struct key *key)
kenter("{%d}", key->serial);
if (rka) {
rcu_assign_keypointer(key, NULL);
- call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
+ request_key_auth_put(rka);
}
}
@@ -174,6 +202,7 @@ struct key *request_key_auth_new(struct key *target, const char *op,
rka = kzalloc_obj(*rka);
if (!rka)
goto error;
+ refcount_set(&rka->usage, 1);
rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
if (!rka->callout_info)
goto error_free_rka;
--
2.47.3
^ permalink raw reply related [flat|nested] 2+ messages in thread* Re: [PATCH] keys: Pin request_key_auth payload in instantiate paths
2026-05-26 2:48 [PATCH] keys: Pin request_key_auth payload in instantiate paths Shaomin Chen
@ 2026-05-29 22:53 ` Jarkko Sakkinen
0 siblings, 0 replies; 2+ messages in thread
From: Jarkko Sakkinen @ 2026-05-29 22:53 UTC (permalink / raw)
To: Shaomin Chen, David Howells
Cc: keyrings, linux-security-module, linux-kernel, David Howells,
Paul Moore, James Morris, Serge E. Hallyn
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
> 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(-)
>
> diff --git a/include/keys/request_key_auth-type.h b/include/keys/request_key_auth-type.h
> index 36b89a933310..01e42ee5f409 100644
> --- a/include/keys/request_key_auth-type.h
> +++ b/include/keys/request_key_auth-type.h
> @@ -9,12 +9,14 @@
> #define _KEYS_REQUEST_KEY_AUTH_TYPE_H
>
> #include <linux/key.h>
> +#include <linux/refcount.h>
>
> /*
> * Authorisation record for request_key().
> */
> struct request_key_auth {
> struct rcu_head rcu;
> + refcount_t usage;
> struct key *target_key;
> struct key *dest_keyring;
> const struct cred *cred;
> diff --git a/security/keys/internal.h b/security/keys/internal.h
> index 2cffa6dc8255..b7b622bc36a1 100644
> --- a/security/keys/internal.h
> +++ b/security/keys/internal.h
> @@ -208,6 +208,8 @@ extern struct key *request_key_auth_new(struct key *target,
> const void *callout_info,
> size_t callout_len,
> struct key *dest_keyring);
> +struct request_key_auth *request_key_auth_get(struct key *authkey);
> +void request_key_auth_put(struct request_key_auth *rka);
>
> extern struct key *key_get_instantiation_authkey(key_serial_t target_id);
>
> diff --git a/security/keys/keyctl.c b/security/keys/keyctl.c
> index ef855d69c97a..d14ace88e529 100644
> --- a/security/keys/keyctl.c
> +++ b/security/keys/keyctl.c
> @@ -1197,9 +1197,13 @@ static long keyctl_instantiate_key_common(key_serial_t id,
> if (!instkey)
> goto error;
>
> - rka = instkey->payload.data[0];
> - if (rka->target_key->serial != id)
> + rka = request_key_auth_get(instkey);
> + if (!rka) {
> + ret = -EKEYREVOKED;
> goto error;
> + }
> + if (rka->target_key->serial != id)
> + goto error_put_rka;
>
> /* pull the payload in if one was supplied */
> payload = NULL;
> @@ -1208,7 +1212,7 @@ static long keyctl_instantiate_key_common(key_serial_t id,
> ret = -ENOMEM;
> payload = kvmalloc(plen, GFP_KERNEL);
> if (!payload)
> - goto error;
> + goto error_put_rka;
>
> ret = -EFAULT;
> if (!copy_from_iter_full(payload, plen, from))
> @@ -1234,6 +1238,8 @@ static long keyctl_instantiate_key_common(key_serial_t id,
>
> error2:
> kvfree_sensitive(payload, plen);
> +error_put_rka:
> + request_key_auth_put(rka);
> error:
> return ret;
> }
> @@ -1358,15 +1364,19 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
> if (!instkey)
> goto error;
>
> - rka = instkey->payload.data[0];
> - if (rka->target_key->serial != id)
> + rka = request_key_auth_get(instkey);
> + if (!rka) {
> + ret = -EKEYREVOKED;
> goto error;
> + }
> + if (rka->target_key->serial != id)
> + goto error_put_rka;
>
> /* find the destination keyring if present (which must also be
> * writable) */
> ret = get_instantiation_keyring(ringid, rka, &dest_keyring);
> if (ret < 0)
> - goto error;
> + goto error_put_rka;
>
> /* instantiate the key and link it into a keyring */
> ret = key_reject_and_link(rka->target_key, timeout, error,
> @@ -1379,6 +1389,8 @@ long keyctl_reject_key(key_serial_t id, unsigned timeout, unsigned error,
> if (ret == 0)
> keyctl_change_reqkey_auth(NULL);
>
> +error_put_rka:
> + request_key_auth_put(rka);
> error:
> return ret;
> }
> diff --git a/security/keys/request_key_auth.c b/security/keys/request_key_auth.c
> index a7d7538c1f70..282e09d8fa46 100644
> --- a/security/keys/request_key_auth.c
> +++ b/security/keys/request_key_auth.c
> @@ -23,6 +23,7 @@ static void request_key_auth_describe(const struct key *, struct seq_file *);
> static void request_key_auth_revoke(struct key *);
> static void request_key_auth_destroy(struct key *);
> static long request_key_auth_read(const struct key *, char *, size_t);
> +static void request_key_auth_rcu_disposal(struct rcu_head *);
>
> /*
> * The request-key authorisation key type definition.
> @@ -115,6 +116,31 @@ static void free_request_key_auth(struct request_key_auth *rka)
> kfree(rka);
> }
>
> +/*
> + * Take a reference to the request-key authorisation payload so callers can
> + * drop authkey->sem before doing operations that may sleep.
> + */
> +struct request_key_auth *request_key_auth_get(struct key *authkey)
> +{
> + struct request_key_auth *rka;
> +
> + down_read(&authkey->sem);
> + rka = dereference_key_locked(authkey);
> + if (rka && !test_bit(KEY_FLAG_REVOKED, &authkey->flags))
> + refcount_inc(&rka->usage);
> + else
> + rka = NULL;
> + up_read(&authkey->sem);
> +
> + return rka;
> +}
> +
> +void request_key_auth_put(struct request_key_auth *rka)
> +{
> + if (rka && refcount_dec_and_test(&rka->usage))
> + call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
> +}
> +
> /*
> * Dispose of the request_key_auth record under RCU conditions
> */
> @@ -136,8 +162,10 @@ static void request_key_auth_revoke(struct key *key)
> struct request_key_auth *rka = dereference_key_locked(key);
>
> kenter("{%d}", key->serial);
> + if (!rka)
> + return;
> rcu_assign_keypointer(key, NULL);
> - call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
> + request_key_auth_put(rka);
> }
>
> /*
> @@ -150,7 +178,7 @@ static void request_key_auth_destroy(struct key *key)
> kenter("{%d}", key->serial);
> if (rka) {
> rcu_assign_keypointer(key, NULL);
> - call_rcu(&rka->rcu, request_key_auth_rcu_disposal);
> + request_key_auth_put(rka);
> }
> }
>
> @@ -174,6 +202,7 @@ struct key *request_key_auth_new(struct key *target, const char *op,
> rka = kzalloc_obj(*rka);
> if (!rka)
> goto error;
> + refcount_set(&rka->usage, 1);
> rka->callout_info = kmemdup(callout_info, callout_len, GFP_KERNEL);
> if (!rka->callout_info)
> goto error_free_rka;
> --
> 2.47.3
I get the approach and for it makes sense but I'd really like to hear
David's feedback on this before moving forward.
In the meanwhile, were you able to do a script or similar as a
reproducer?
BR, Jarkko
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2026-05-29 22:53 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-26 2:48 [PATCH] keys: Pin request_key_auth payload in instantiate paths Shaomin Chen
2026-05-29 22:53 ` Jarkko Sakkinen
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox