Re: [PATCH 1/1] audit: fix potential use-after-free in audit_del_rule()

Paul Moore <[email protected]> =E4=BA=8E2026=E5=B9=B47=E6=9C=8824=E6=97=
=A5=E5=91=A8=E4=BA=94 05:57=E5=86=99=E9=81=93=EF=BC=9A
>
> 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 fsnot=
ify
> > mark can be freed on an independent lifetime path. This creates a use-a=
fter-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 othe=
r
> > 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.
>

Thanks for reviewing the patch and for the suggestion! I did look
into using call_rcu() for the cleanup, but we cannot do so due to
sleeping constraints in RCU callbacks.

Specifically, the cleanup helpers called during rule deletion (such as
audit_remove_mark_rule(), audit_remove_watch_rule(), and
audit_remove_tree_rule()) can sleep. For instance, removing a mark via
audit_remove_mark_rule() eventually calls fsnotify_destroy_mark(),
which acquires mutexes (e.g., group->mark_mutex) and can block.

Since call_rcu() callbacks run in softirq (atomic) context, invoking
these sleeping functions there would trigger a "scheduling while atomic"
bug.

audit_del_rule() runs in process context, which allows it to safely
sleep during synchronize_rcu() and the subsequent rule/mark removals.
Therefore, synchronize_rcu() appears to be the necessary choice here.


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