mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v2] audit: fix exe mark UAF in kill_rules()
@ 2026-09-22 20:27 Jérémy Jean
  2026-09-22 20:31 ` Bradley Morgan
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jérémy Jean @ 2026-09-22 20:27 UTC (permalink / raw)
  To: Paul Moore, Eric Paris
  Cc: rrobaina, brads, audit, linux-kernel, Jérémy Jean

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 <Jeremy.Jean@oss.cyber.gouv.fr>
---

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/20260921195921.4174830-2-Jeremy.Jean@oss.cyber.gouv.fr/

 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);
+	}
 }
 
 /*

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] audit: fix exe mark UAF in kill_rules()
  2026-09-22 20:27 [PATCH v2] audit: fix exe mark UAF in kill_rules() Jérémy Jean
@ 2026-09-22 20:31 ` Bradley Morgan
  2026-09-23 11:55 ` Ricardo Robaina
  2026-09-25 20:18 ` Paul Moore
  2 siblings, 0 replies; 4+ messages in thread
From: Bradley Morgan @ 2026-09-22 20:31 UTC (permalink / raw)
  To: Jérémy Jean, Paul Moore, Eric Paris
  Cc: rrobaina, audit, linux-kernel

On 22 September 2026 21:27:35 BST, "Jérémy Jean"
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>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

Reviewed-by: Bradley Morgan <brads@mainlining.org>


>Signed-off-by: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>
>---
>
>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/20260921195921.4174830-2-Jeremy.Jean@oss.cyber.gouv.fr/
>
> 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);
>+	}
> }
> 
> /*

--- Thanks!
"I'm not a very positive person" - Linus torvalds

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] audit: fix exe mark UAF in kill_rules()
  2026-09-22 20:27 [PATCH v2] audit: fix exe mark UAF in kill_rules() Jérémy Jean
  2026-09-22 20:31 ` Bradley Morgan
@ 2026-09-23 11:55 ` Ricardo Robaina
  2026-09-25 20:18 ` Paul Moore
  2 siblings, 0 replies; 4+ messages in thread
From: Ricardo Robaina @ 2026-09-23 11:55 UTC (permalink / raw)
  To: Jérémy Jean; +Cc: Paul Moore, Eric Paris, brads, audit, linux-kernel

On Tue, Sep 22, 2026 at 5:34 PM Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> wrote:
>
> 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 <Jeremy.Jean@oss.cyber.gouv.fr>

Looks good to me. I've checked that audit-testsuite passes cleanly.

Reviewed-by: Ricardo Robaina <rrobaina@redhat.com>
Tested-by: Ricardo Robaina <rrobaina@redhat.com>

# uname -r
7.3.0-rc4+

# make test
make -C tests test
Running as   user    root
        with context unconfined_u:unconfined_r:unconfined_t:s0-s0:c0.c1023
        on   system  Fedora

amcast_joinpart/test ................. ok
backlog_wait_time_actual_reset/test .. ok
bpf/test ............................. ok
coredump/test ........................ ok
exec_execve/test ..................... ok
exec_name/test ....................... ok
fanotify/test ........................ ok
field_compare/test ................... ok
file_create/test ..................... ok
file_delete/test ..................... ok
file_permission/test ................. ok
file_rename/test ..................... ok
filter_device/test ................... ok
filter_exclude/test .................. ok
filter_exit/test ..................... ok
filter_inode/test .................... ok
filter_saddr_fam/test ................ ok
filter_sessionid/test ................ ok
io_uring/test ........................ ok
login_tty/test ....................... ok
lost_reset/test ...................... ok
netfilter_pkt/test ................... ok
signal/test .......................... ok
syscalls_file/test ................... ok
syscall_module/test .................. ok
syscall_socketcall/test .............. ok
time_change/test ..................... ok
user_msg/test ........................ ok
All tests successful.
Files=28, Tests=303, 69 wallclock secs ( 0.08 usr  0.03 sys + 36.64
cusr  1.87 csys = 38.62 CPU)
Result: PASS


> ---
>
> 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/20260921195921.4174830-2-Jeremy.Jean@oss.cyber.gouv.fr/
>
>  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);
> +       }
>  }
>
>  /*
>

--Ricardo


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH v2] audit: fix exe mark UAF in kill_rules()
  2026-09-22 20:27 [PATCH v2] audit: fix exe mark UAF in kill_rules() Jérémy Jean
  2026-09-22 20:31 ` Bradley Morgan
  2026-09-23 11:55 ` Ricardo Robaina
@ 2026-09-25 20:18 ` Paul Moore
  2 siblings, 0 replies; 4+ messages in thread
From: Paul Moore @ 2026-09-25 20:18 UTC (permalink / raw)
  To: Jérémy Jean, Eric Paris
  Cc: rrobaina, brads, audit, linux-kernel, Jérémy Jean

On Sep 22, 2026 =?UTF-8?q?J=C3=A9r=C3=A9my=20Jean?= <Jeremy.Jean@oss.cyber.gouv.fr> wrote:
> 
> 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 <Jeremy.Jean@oss.cyber.gouv.fr>
> Reviewed-by: Ricardo Robaina <rrobaina@redhat.com>
> Tested-by: Ricardo Robaina <rrobaina@redhat.com>
> Reviewed-by: Bradley Morgan <brads@mainlining.org>
> ---
> 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/20260921195921.4174830-2-Jeremy.Jean@oss.cyber.gouv.fr/
> 
>  kernel/audit_tree.c | 24 ++++++++++++++++++++----
>  1 file changed, 20 insertions(+), 4 deletions(-)

Merged into audit/stable-7.3 with a stable tag, I'll send this up to Linus
next week after it has had some time in linux-next.  Thanks!

--
paul-moore.com

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-09-25 20:18 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-22 20:27 [PATCH v2] audit: fix exe mark UAF in kill_rules() Jérémy Jean
2026-09-22 20:31 ` Bradley Morgan
2026-09-23 11:55 ` Ricardo Robaina
2026-09-25 20:18 ` Paul Moore

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®