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]>
