AppArmor's begin_current_label_crit_section() is a scary function called
from lots of LSM hooks (in particular VFS/socket-related ones) that checks
if the label referenced by the current creds is marked FLAG_STALE, and if
so, attempts to use aa_replace_current_label() to replace the creds with an
updated version that uses a new label.
The first problem with this is that it would directly lead to UAF of
`struct cred` if anything in the kernel takes a pointer to the current
creds and accesses these past a security hook invocation that replaces
creds, like so:
```
const struct cred *cred = current_cred();
alloc_file_pseudo(...);
uid_t uid = cred->euid;
```
I don't know if anything in the kernel actually does this, but I think it
is very surprising that this pattern could lead to UAF.
The second problem is that things go wrong when aa_replace_current_label()
runs with overridden credentials. aa_replace_current_label() bails out if
`current_cred() != current_real_cred()` (mirroring the check in
proc_pid_attr_write()), but this check can't actually reliably detect
overridden credentials because the overridden creds can be the same as the
objective creds.
So in approximately the following scenario, things go wrong:
1. task begins with <creds A> (as both objective and subjective creds),
with refcount=2
2. task grabs an extra reference on <creds A> for overriding
3. task calls override_creds(<creds A>), which returns a pointer to the old
subjective creds (<creds A>)
4. task enters AppArmor LSM hook
5. AppArmor checks that objective/subjective creds are equal
6. AppArmor replaces both cred pointers with <creds B> and drops 2 refs on
<creds A>
7. task leaves AppArmor LSM hook
8. task calls revert_creds(<creds A>)
9. now task->cred is <creds A> while task->real_cred is <creds B>, but the
task_struct logically holds two references to <creds B>
10. another task drops the extra reference on <creds A> that was used for
overriding, refcount drops to 0
11. now task->real_cred points to freed creds
At this point, any access to current_cred() will be UAF.
I have a test case where I run aa-disable on a profile while a process
using that profile is blocked on splice() from a FUSE passthrough file into
a full pipe; after the profile update, the pipe becomes empty, splice()
resumes, the credentials go out of sync, and a subsequent getuid() syscall
results in a KASAN UAF splat.
To fix this, instead of directly replacing creds, do it via task_work that
will run at the end of the current syscall. (The point in time at which the
cred replacement happens should have no correctness impact; it is just a
performance optimization to avoid unnecessarily touching the refcount of
the new label.)
Note that AppArmor still performs direct cred replacements in the
sb_pivotroot LSM hook after this change, and that direct cred replacements
can still happen in VFS ->write() callbacks via proc_pid_attr_write().
Cc: [email protected]
Fixes: c75afcd153f6 ("AppArmor: contexts used in attaching policy to system
objects")
Signed-off-by: Jann Horn <[email protected]>
---
include/linux/task_work.h | 1 +
kernel/task_work.c | 14 ++++++++++++++
security/apparmor/include/cred.h | 6 +-----
security/apparmor/include/task.h | 1 +
security/apparmor/task.c | 29 +++++++++++++++++++++++++++++
5 files changed, 46 insertions(+), 5 deletions(-)
diff --git a/include/linux/task_work.h b/include/linux/task_work.h
index 0646804860ff..ce19fc14060c 100644
--- a/include/linux/task_work.h
+++ b/include/linux/task_work.h
@@ -33,6 +33,7 @@ struct callback_head *task_work_cancel_match(struct
task_struct *task,
bool (*match)(struct callback_head *, void *data), void *data);
struct callback_head *task_work_cancel_func(struct task_struct *,
task_work_func_t);
bool task_work_cancel(struct task_struct *task, struct callback_head *cb);
+bool task_work_has_func(struct task_struct *task, task_work_func_t func);
void task_work_run(void);
static inline void exit_task_work(struct task_struct *task)
diff --git a/kernel/task_work.c b/kernel/task_work.c
index 0f7519f8e7c9..f83d1528e0bc 100644
--- a/kernel/task_work.c
+++ b/kernel/task_work.c
@@ -189,6 +189,20 @@ bool task_work_cancel(struct task_struct *task, struct
callback_head *cb)
return ret == cb;
}
+bool task_work_has_func(struct task_struct *task, task_work_func_t func)
+{
+ struct callback_head *work;
+
+ if (!task_work_pending(task))
+ return false;
+ guard(raw_spinlock_irqsave)(&task->pi_lock);
+ for (work = READ_ONCE(task->task_works); work; work =
READ_ONCE(work->next)) {
+ if (work->func == func)
+ return true;
+ }
+ return false;
+}
+
/**
* task_work_run - execute the works added by task_work_add()
*
diff --git a/security/apparmor/include/cred.h b/security/apparmor/include/cred.h
index 2b6098149b15..0e8b67159f56 100644
--- a/security/apparmor/include/cred.h
+++ b/security/apparmor/include/cred.h
@@ -222,13 +222,9 @@ static inline struct aa_label
*begin_current_label_crit_section(void)
{
struct aa_label *label = aa_current_raw_label();
- might_sleep();
-
if (label_is_stale(label)) {
label = aa_get_newest_label(label);
- if (aa_replace_current_label(label) == 0)
- /* task cred will keep the reference */
- aa_put_label(label);
+ aa_schedule_stale_label_replacement();
}
return label;
diff --git a/security/apparmor/include/task.h b/security/apparmor/include/task.h
index b1aaaf60fa8b..4e49a4142777 100644
--- a/security/apparmor/include/task.h
+++ b/security/apparmor/include/task.h
@@ -30,6 +30,7 @@ struct aa_task_ctx {
};
int aa_replace_current_label(struct aa_label *label);
+void aa_schedule_stale_label_replacement(void);
void aa_set_current_onexec(struct aa_label *label, bool stack);
int aa_set_current_hat(struct aa_label *label, u64 token);
int aa_restore_previous_label(u64 cookie);
diff --git a/security/apparmor/task.c b/security/apparmor/task.c
index b9fb3738124e..8e368f6278f5 100644
--- a/security/apparmor/task.c
+++ b/security/apparmor/task.c
@@ -14,6 +14,7 @@
#include <linux/gfp.h>
#include <linux/ptrace.h>
+#include <linux/task_work.h>
#include "include/path.h"
#include "include/audit.h"
@@ -89,6 +90,34 @@ int aa_replace_current_label(struct aa_label *label)
return 0;
}
+static void aa_replace_stale_label_tw_func(struct callback_head *tw)
+{
+ struct aa_label *label;
+
+ kfree(tw);
+ label = aa_current_raw_label();
+ if (!label_is_stale(label))
+ return;
+ label = aa_get_newest_label(label);
+ aa_replace_current_label(label);
+ aa_put_label(label);
+}
+
+/* replace the current task's stale label on syscall return */
+void aa_schedule_stale_label_replacement(void)
+{
+ struct callback_head *tw;
+
+ if (task_work_has_func(current, aa_replace_stale_label_tw_func))
+ return;
+ tw = kmalloc_obj(struct callback_head);
+ if (!tw)
+ return;
+ init_task_work(tw, aa_replace_stale_label_tw_func);
+ if (task_work_add(current, tw, TWA_RESUME))
+ kfree(tw);
+}
+
/**
* aa_set_current_onexec - set the tasks change_profile to happen onexec
---
base-commit: 3b029c035b34bbc693405ddf759f0e9b920c27f1
change-id: 20260714-fix-apparmor-cred-uaf-cc38ec2b38b7
Best regards,
--
Jann Horn <[email protected]>
