Re: [PATCH 1/1] audit: fix potential use-after-free in audit_del_rule()
Paul Moore <[email protected]> Thu, 23 Jul 2026 17:57:52 -0400
| Newsgroups | org.kernel.vger.audit |
|---|---|
| Message-ID | <[email protected]> |
On Jul 21, 2026 Ren Wei <[email protected]> wrote: > > `audit_del_rule()` destroys `e->rule.exe` via `audit_remove_mark_rule()` > before unlinking the rule from RCU-visible filter lists and waiting for a > grace period. Concurrent readers in `audit_filter()` and > `audit_filter_rules()` still dereference `e->rule.exe`, while the fsnotify > mark can be freed on an independent lifetime path. This creates a use-after-free > window during rule deletion. > > Fix this by unlinking the rule from the RCU-visible lists and invoking > `synchronize_rcu()` before calling `audit_remove_mark_rule()` (and other > rule removal helpers). This ensures that all existing RCU readers have exited > the critical section before any underlying resources are destroyed. > > Fixes: 34d99af52ad4 ("audit: implement audit by executable") > Cc: [email protected] > Reported-by: Vega <[email protected]> > Assisted-by: Codex:gpt-5.4 > Signed-off-by: Luxiao Xu <[email protected]> > Signed-off-by: Ren Wei <[email protected]> > --- > kernel/auditfilter.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) Thanks for identifying this and providing a patch, some initial thoughts below ... > diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c > index 4401119b5275..7f791afe5791 100644 > --- a/kernel/auditfilter.c > +++ b/kernel/auditfilter.c > @@ -1045,6 +1045,10 @@ int audit_del_rule(struct audit_entry *entry) > goto out; > } > > + list_del_rcu(&e->list); > + list_del(&e->rule.list); > + synchronize_rcu(); I generally prefer to use call_rcu() instead of synchronize_rcu() when possible. Did you consider creating a audit_del_rule_rcu() (or similar) function which would perform the other cleanups/releases below this point including the call to audit_free_rule_rcu() which could then be called by call_rcu()? It's worth mentioning that I didn't verify all of the code involved to know if this would work, be safe, etc., I just wanted to check to see if you already had and decided synchronize_rcu() was the only option. > if (e->rule.watch) > audit_remove_watch_rule(&e->rule); > > @@ -1062,8 +1066,6 @@ int audit_del_rule(struct audit_entry *entry) > audit_signals--; > #endif > > - list_del_rcu(&e->list); > - list_del(&e->rule.list); > call_rcu(&e->rcu, audit_free_rule_rcu); > > out: > -- > 2.43.0 -- paul-moore.com