[PATCH] cred: clarify that task_struct::cred is only for the current task

[PATCH] cred: clarify that task_struct::cred is only for the current task

[Jann Horn]

The `cred` field in task_struct is currently marked as __rcu, but that's
not true: It can point to credentials from access_override_creds(), which
do not get freed with RCU delay.

What actually protects task_struct::cred is that accessing it is only
permitted for the current task (except for setting up a child during fork()
or tearing down a dead process).
(There is currently code in Smack that violates this rule, but that's a bug
and causes UAF, I have sent a separate fix for that.)

Clarify this, remove the __rcu marker, and remove RCU helpers from all
accesses to this field.

Signed-off-by: Jann Horn <jannh@google.com>
---
For context:
There have been at least two UAFs of struct cred that I'm aware of, both
caused by wrong use of task_struct::cred:

 - https://git.kernel.org/linus/a3727a8bac0a9e77c70820655fd8715523ba3db7
 - https://lore.kernel.org/all/20260806-smack-uaf-fix-v1-1-26426d389a26@google.com/
---
 include/linux/cred.h  | 17 +++++++++++------
 include/linux/sched.h |  8 ++++++--
 kernel/auditsc.c      |  5 +++--
 kernel/cred.c         |  2 +-
 security/lsm_init.c   |  2 +-
 5 files changed, 22 insertions(+), 12 deletions(-)

diff --git a/include/linux/cred.h b/include/linux/cred.h
index c6676265a985..650aefd1416a 100644
--- a/include/linux/cred.h
+++ b/include/linux/cred.h
@@ -180,12 +180,18 @@ static inline bool cap_ambient_invariant_ok(const struct cred *cred)
 
 static inline const struct cred *override_creds(const struct cred *override_cred)
 {
-	return rcu_replace_pointer(current->cred, override_cred, 1);
+	const struct cred *old = current->cred;
+
+	current->cred = override_cred;
+	return old;
 }
 
 static inline const struct cred *revert_creds(const struct cred *revert_cred)
 {
-	return rcu_replace_pointer(current->cred, revert_cred, 1);
+	const struct cred *override_cred = current->cred;
+
+	current->cred = revert_cred;
+	return override_cred;
 }
 
 DEFINE_CLASS(override_creds,
@@ -293,11 +299,10 @@ DEFINE_FREE(put_cred, struct cred *, if (!IS_ERR_OR_NULL(_T)) put_cred(_T))
 /**
  * current_cred - Access the current task's subjective credentials
  *
- * Access the subjective credentials of the current task.  RCU-safe,
- * since nobody else can modify it.
+ * Access the subjective credentials of the current task.
+ * Nobody else can modify it.
  */
-#define current_cred() \
-	rcu_dereference_protected(current->cred, 1)
+#define current_cred() (current->cred)
 
 /**
  * current_real_cred - Access the current task's objective credentials
diff --git a/include/linux/sched.h b/include/linux/sched.h
index 373bcc0598d1..6f489c2cf5ea 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1163,8 +1163,12 @@ struct task_struct {
 	/* Objective and real subjective task credentials (COW): */
 	const struct cred __rcu		*real_cred;
 
-	/* Effective (overridable) subjective task credentials (COW): */
-	const struct cred __rcu		*cred;
+	/*
+	 * Effective (overridable) subjective task credentials (COW).
+	 * Only accessible for the current task and during task creation/freeing.
+	 * This pointer is not managed by RCU!
+	 */
+	const struct cred		*cred;
 
 #ifdef CONFIG_KEYS
 	/* Cached requested key. */
diff --git a/kernel/auditsc.c b/kernel/auditsc.c
index 6610e667c728..646145a66196 100644
--- a/kernel/auditsc.c
+++ b/kernel/auditsc.c
@@ -459,7 +459,7 @@ static int audit_field_compare(struct task_struct *tsk,
  *
  * If task_creation is true, this is an explicit indication that we are
  * filtering a task rule at task creation time.  This and tsk == current are
- * the only situations where tsk->cred may be accessed without an rcu read lock.
+ * the only situations where tsk->cred may be accessed.
  */
 static int audit_filter_rules(struct task_struct *tsk,
 			      struct audit_krule *rule,
@@ -476,7 +476,8 @@ static int audit_filter_rules(struct task_struct *tsk,
 	if (ctx && rule->prio <= ctx->prio)
 		return 0;
 
-	cred = rcu_dereference_check(tsk->cred, tsk == current || task_creation);
+	WARN_ON(tsk != current && !task_creation);
+	cred = tsk->cred;
 
 	for (i = 0; i < rule->field_count; i++) {
 		struct audit_field *f = &rule->fields[i];
diff --git a/kernel/cred.c b/kernel/cred.c
index 3df4e15bd67f..0bd6a58bc12d 100644
--- a/kernel/cred.c
+++ b/kernel/cred.c
@@ -414,7 +414,7 @@ int commit_creds(struct cred *new)
 		inc_rlimit_ucounts(new->ucounts, UCOUNT_RLIMIT_NPROC, 1);
 
 	rcu_assign_pointer(task->real_cred, new);
-	rcu_assign_pointer(task->cred, new);
+	task->cred = new;
 	if (new->user != old->user || new->user_ns != old->user_ns)
 		dec_rlimit_ucounts(old->ucounts, UCOUNT_RLIMIT_NPROC, 1);
 	if (new->user_ns != old->user_ns)
diff --git a/security/lsm_init.c b/security/lsm_init.c
index 7c0fd17f1601..1328a2ceef4b 100644
--- a/security/lsm_init.c
+++ b/security/lsm_init.c
@@ -476,7 +476,7 @@ int __init security_init(void)
 						    blob_sizes.lbs_inode, 0,
 						    SLAB_PANIC, NULL);
 
-	if (lsm_cred_alloc((struct cred *)unrcu_pointer(current->cred),
+	if (lsm_cred_alloc((struct cred *)current->cred,
 			   GFP_KERNEL))
 		panic("early LSM cred alloc failed\n");
 	if (lsm_task_alloc(current))

---
base-commit: fcaeecb8b0cd44f77d03b28de0671258d4db18f8
change-id: 20260806-cred-nonrcu-annotation-fix-ab0b7427b310

Best regards,
--  
Jann Horn <jannh@google.com>

Re: [PATCH] cred: clarify that task_struct::cred is only for the current task

[Paul Moore]

On Aug  6, 2026 Jann Horn <jannh@google.com> wrote:
> 
> The `cred` field in task_struct is currently marked as __rcu, but that's
> not true: It can point to credentials from access_override_creds(), which
> do not get freed with RCU delay.
> 
> What actually protects task_struct::cred is that accessing it is only
> permitted for the current task (except for setting up a child during fork()
> or tearing down a dead process).
> (There is currently code in Smack that violates this rule, but that's a bug
> and causes UAF, I have sent a separate fix for that.)
> 
> Clarify this, remove the __rcu marker, and remove RCU helpers from all
> accesses to this field.
> 
> Signed-off-by: Jann Horn <jannh@google.com>
> ---
> For context:
> There have been at least two UAFs of struct cred that I'm aware of, both
> caused by wrong use of task_struct::cred:
> 
>  - https://git.kernel.org/linus/a3727a8bac0a9e77c70820655fd8715523ba3db7
>  - https://lore.kernel.org/all/20260806-smack-uaf-fix-v1-1-26426d389a26@google.com/
> ---
>  include/linux/cred.h  | 17 +++++++++++------
>  include/linux/sched.h |  8 ++++++--
>  kernel/auditsc.c      |  5 +++--
>  kernel/cred.c         |  2 +-
>  security/lsm_init.c   |  2 +-
>  5 files changed, 22 insertions(+), 12 deletions(-)

This looks good to me, with one small nit below, but as we are at -rc7
I think this is best kept until after the upcoming merge window so it
has more than a few days in linux-next.  I'm going to merge this into
lsm/dev-staging with the expectation of merging it into lsm/dev after
the upcoming merge window closes.  If anyone wishes to object to this
patch or add their own review/ACK/test tag before then, please do.

Thanks Jann.

> diff --git a/security/lsm_init.c b/security/lsm_init.c
> index 7c0fd17f1601..1328a2ceef4b 100644
> --- a/security/lsm_init.c
> +++ b/security/lsm_init.c
> @@ -476,7 +476,7 @@ int __init security_init(void)
>  						    blob_sizes.lbs_inode, 0,
>  						    SLAB_PANIC, NULL);
>  
> -	if (lsm_cred_alloc((struct cred *)unrcu_pointer(current->cred),
> +	if (lsm_cred_alloc((struct cred *)current->cred,
>  			   GFP_KERNEL))

Just a FYI, but I pulled the GFP_KERNEL up on the line above while
merging since it now fits.

--
paul-moore.com