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

Jann Horn <[email protected]>
Newsgroups org.kernel.vger.linux-security-module,org.kernel.vger.audit,org.kernel.vger.linux-kernel
Message-ID <[email protected]>
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 <[email protected]>
---
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/[email protected]/
---
 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 <[email protected]>
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.