Re: [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove

[Ricardo Robaina]

On Tue, Aug 11, 2026 at 7:02 PM Jérémy Jean
<Jeremy.Jean@xxxxxxxxxxxxxxxxx> wrote:
>
> audit_del_rule() is used for both netlink deletion templates and internal
> fsnotify autoremove.  The former passes a parsed template which owns a
> temporary tree reference; the latter passes the installed entry itself.
>
> The unconditional audit_put_tree() at the end of audit_del_rule() assumes
> the template case.  For mixed AUDIT_DIR plus AUDIT_EXE rules, an fsnotify
> autoremove event therefore drops the installed rule's live tree reference.
> Repeating this across rules sharing the same tree can free the tree while
> another rule still references it, and a later autoremove dereferences the
> freed pathname while comparing rules.
>
> Move the temporary-tree put to audit_rule_change(), the caller that owns
> deletion templates.  Keep it in the AUDIT_DEL_RULE cleanup so both
> successful deletion and -ENOENT still release the parser-owned tree.
>
> Fixes: 34d99af52ad4 ("audit: implement audit by executable")
> Assisted-by: Codex:gpt-5
> Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> ---
>  kernel/auditfilter.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
>
> diff --git a/kernel/auditfilter.c b/kernel/auditfilter.c
> index 7f791afe5791..666c2091b9e4 100644
> --- a/kernel/auditfilter.c
> +++ b/kernel/auditfilter.c
> @@ -1023,7 +1023,6 @@ static inline int audit_add_rule(struct audit_entry *entry)
>  int audit_del_rule(struct audit_entry *entry)
>  {
>         struct audit_entry  *e;
> -       struct audit_tree *tree = entry->rule.tree;
>         struct list_head *list;
>         int ret = 0;
>  #ifdef CONFIG_AUDITSYSCALL
> @@ -1071,9 +1070,6 @@ int audit_del_rule(struct audit_entry *entry)
>  out:
>         mutex_unlock(&audit_filter_mutex);
>
> -       if (tree)
> -               audit_put_tree(tree);   /* that's the temporary one */
> -
>         return ret;
>  }
>
> @@ -1158,6 +1154,8 @@ int audit_rule_change(int type, int seq, void *data, size_t datasz)
>         }
>
>         if (err || type == AUDIT_DEL_RULE) {
> +               if (type == AUDIT_DEL_RULE && entry->rule.tree)
> +                       audit_put_tree(entry->rule.tree); /* that's the template one */
>                 if (entry->rule.exe)
>                         audit_remove_mark(entry->rule.exe);
>                 audit_free_rule(entry);
> --
> 2.47.3
>
>

Looks good to me. It passes the audit testsuite as well.

# make test
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, 107 wallclock secs ( 0.10 usr  0.02 sys + 31.68
cusr  1.89 csys = 33.69 CPU)
Result: PASS

Reviewed-by: Ricardo Robaina <rrobaina@xxxxxxxxxx>
Tested-by: Ricardo Robaina <rrobaina@xxxxxxxxxx>

-Ricardo