kill_rules() removes mixed AUDIT_DIR and AUDIT_EXE rules when an audit
tree is pruned. It drops entry->rule.exe before removing the rule from
the RCU-visible filter lists.

After a rule has been installed with AUDIT_ADD_RULE, which requires
CAP_AUDIT_CONTROL, removing the watched directory can race with another
task that is still evaluating the rule. In that case, fsnotify can free
the executable mark before the reader reaches audit_mark_compare(),
causing a use-after-free.

KASAN reports:

    BUG: KASAN: slab-use-after-free in audit_mark_compare+0x8d/0xa0

Unlink the published rules from the RCU-visible lists and retain them
on tree->rules for cleanup. If any rule has an executable mark, wait for
a single RCU grace period before removing the marks and scheduling the
entries for freeing. Otherwise, call_rcu() already provides the required
deferred freeing without a synchronous wait.

Fixes: 34d99af52ad4 ("audit: implement audit by executable")
Assisted-by: Codex:gpt-5
Signed-off-by: Jérémy Jean <[email protected]>
---

v2: Address Sashiko's review with Ricardo Robaina's improved patch:
- Batch executable-mark teardown behind one synchronize_rcu() after
  unlinking all rules, instead of waiting once per rule under the audit
  mutexes.
- Skip the synchronous wait when none of the removed rules has an
  executable mark.

v1: 
https://lore.kernel.org/all/[email protected]/

 kernel/audit_tree.c | 24 ++++++++++++++++++++----
 1 file changed, 20 insertions(+), 4 deletions(-)

diff --git a/kernel/audit_tree.c b/kernel/audit_tree.c
index 1ed19b775912..f2e81be8265e 100644
--- a/kernel/audit_tree.c
+++ b/kernel/audit_tree.c
@@ -545,22 +545,38 @@ static void kill_rules(struct audit_context *context, 
struct audit_tree *tree)
 {
        struct audit_krule *rule, *next;
        struct audit_entry *entry;
+       bool need_sync = false;
 
        list_for_each_entry_safe(rule, next, &tree->rules, rlist) {
                entry = container_of(rule, struct audit_entry, rule);
 
-               list_del_init(&rule->rlist);
                if (rule->tree) {
                        /* not a half-baked one */
                        audit_tree_log_remove_rule(context, rule);
-                       if (entry->rule.exe)
-                               audit_remove_mark(entry->rule.exe);
                        rule->tree = NULL;
                        list_del_rcu(&entry->list);
                        list_del(&entry->rule.list);
-                       call_rcu(&entry->rcu, audit_free_rule_rcu);
+                       if (entry->rule.exe)
+                               need_sync = true;
+               } else {
+                       list_del_init(&rule->rlist);
                }
        }
+
+       if (list_empty(&tree->rules))
+               return;
+
+       if (need_sync)
+               synchronize_rcu();
+
+       list_for_each_entry_safe(rule, next, &tree->rules, rlist) {
+               entry = container_of(rule, struct audit_entry, rule);
+
+               list_del_init(&rule->rlist);
+               if (entry->rule.exe)
+                       audit_remove_mark(entry->rule.exe);
+               call_rcu(&entry->rcu, audit_free_rule_rcu);
+       }
 }
 
 /*

Reply via email to