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