mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove
@ 2026-08-11 22:01 Jérémy Jean
  2026-08-12 15:59 ` Ricardo Robaina
                   ` (2 more replies)
  0 siblings, 3 replies; 4+ messages in thread
From: Jérémy Jean @ 2026-08-11 22:01 UTC (permalink / raw)
  To: audit; +Cc: Paul Moore, Eric Paris, linux-kernel, Jérémy Jean

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


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

* Re: [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove
  2026-08-11 22:01 [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove Jérémy Jean
@ 2026-08-12 15:59 ` Ricardo Robaina
  2026-08-12 20:46 ` Paul Moore
  2026-08-13 18:38 ` Richard Guy Briggs
  2 siblings, 0 replies; 4+ messages in thread
From: Ricardo Robaina @ 2026-08-12 15:59 UTC (permalink / raw)
  To: Jérémy Jean; +Cc: audit, Paul Moore, Eric Paris, linux-kernel

On Tue, Aug 11, 2026 at 7:02 PM Jérémy Jean
<Jeremy.Jean@oss.cyber.gouv.fr> 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@oss.cyber.gouv.fr>
> ---
>  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@redhat.com>
Tested-by: Ricardo Robaina <rrobaina@redhat.com>

-Ricardo


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

* Re: [PATCH] audit: avoid dropping live tree ref on fsnotify rule  autoremove
  2026-08-11 22:01 [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove Jérémy Jean
  2026-08-12 15:59 ` Ricardo Robaina
@ 2026-08-12 20:46 ` Paul Moore
  2026-08-13 18:38 ` Richard Guy Briggs
  2 siblings, 0 replies; 4+ messages in thread
From: Paul Moore @ 2026-08-12 20:46 UTC (permalink / raw)
  To: Jérémy Jean, audit
  Cc: Eric Paris, linux-kernel, Jérémy Jean

On Aug 11, 2026 =?UTF-8?q?J=C3=A9r=C3=A9my=20Jean?= <Jeremy.Jean@oss.cyber.gouv.fr> 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@oss.cyber.gouv.fr>
> Reviewed-by: Ricardo Robaina <rrobaina@redhat.com>
> Tested-by: Ricardo Robaina <rrobaina@redhat.com>
> ---
>  kernel/auditfilter.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)

Thanks for finding this and submitting a fix!

This looks good to me too, so I'm going to merge this, but instead of the
normal audit/stable-7.2 branch, I'm going to merge this into the audit/dev
branch since we are likely only a few days away from a v7.2 release and I
think it would be good to get some additional testing.  I am going to tag
this commit with a stable tag so it will/should get backported once it
lands in Linus' tree.

--
paul-moore.com

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

* Re: [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove
  2026-08-11 22:01 [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove Jérémy Jean
  2026-08-12 15:59 ` Ricardo Robaina
  2026-08-12 20:46 ` Paul Moore
@ 2026-08-13 18:38 ` Richard Guy Briggs
  2 siblings, 0 replies; 4+ messages in thread
From: Richard Guy Briggs @ 2026-08-13 18:38 UTC (permalink / raw)
  To: Jérémy Jean; +Cc: audit, Paul Moore, Eric Paris, linux-kernel

On 2026-08-11 22:01, Jérémy Jean 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@oss.cyber.gouv.fr>

Thanks for finding this!

Reviewed-by: Richard Guy Briggs <rgb@redhat.com>

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

- RGB

--
Richard Guy Briggs <rgb@redhat.com>
Sr. S/W Engineer, Kernel Security, Base Operating Systems
Remote, Ottawa, Red Hat Canada
Upstream IRC: SunRaycer
Voice: +1.613.860 2354 SMS: +1.613.518.6570


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

end of thread, other threads:[~2026-08-13 18:39 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-08-11 22:01 [PATCH] audit: avoid dropping live tree ref on fsnotify rule autoremove Jérémy Jean
2026-08-12 15:59 ` Ricardo Robaina
2026-08-12 20:46 ` Paul Moore
2026-08-13 18:38 ` Richard Guy Briggs

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®