linux-kernel - Re: [PATCH] fanotify: Don't call fsnotify_destroy_group() when fsnotify_alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <xlywujl6hhu6nqntnimdcdwbh2okcqxknwzlt5j3p7kqi5uuy7@x3pkdhfkpwf7>
Date: Fri, 28 Nov 2025 12:30:03 +0100
From: Jan Kara <jack@...e.cz>
To: Kuniyuki Iwashima <kuniyu@...gle.com>
Cc: Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, 
	Amir Goldstein <amir73il@...il.com>, Matthew Bobrowski <repnop@...gle.com>, 
	Kuniyuki Iwashima <kuni1840@...il.com>, linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org, 
	syzbot+321168dfa622eda99689@...kaller.appspotmail.com
Subject: Re: [PATCH] fanotify: Don't call fsnotify_destroy_group() when
 fsnotify_alloc_group() fails.

On Thu 27-11-25 20:16:15, Kuniyuki Iwashima wrote:
> syzbot reported the splat in __do_sys_fanotify_init(). [0]
> 
> The cited commit introduced the fsnotify_group class.
> 
> The constructor is fsnotify_alloc_group() and could fail,
> so the error is handled this way:
> 
> 	CLASS(fsnotify_group, group)(&fanotify_fsnotify_ops,
> 				     FSNOTIFY_GROUP_USER);
> 	if (IS_ERR(group))
> 		return PTR_ERR(group);
> 
> Even we return from the path, the destructor is triggered,
> and the condition does not take IS_ERR() into account.
> 
> 	if (_T) fsnotify_destroy_group(_T),
> 
> Thus, fsnotify_destroy_group() could be called for ERR_PTR().
> 
> Let's fix the condition to !IS_ERR_OR_NULL(_T).
> 
> [0]:
> Oops: general protection fault, probably for non-canonical address 0xdffffc0000000003: 0000 [#1] SMP KASAN PTI
> KASAN: null-ptr-deref in range [0x0000000000000018-0x000000000000001f]
> CPU: 1 UID: 0 PID: 6016 Comm: syz.0.17 Not tainted syzkaller #0 PREEMPT(full)
> Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 10/25/2025
> RIP: 0010:kasan_byte_accessible+0x12/0x30 mm/kasan/generic.c:210
> Code: 0f 1f 84 00 00 00 00 00 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 90 0f 1f 40 d6 48 c1 ef 03 48 b8 00 00 00 00 00 fc ff df <0f> b6 04 07 3c 08 0f 92 c0 e9 40 01 33 09 cc 66 66 66 66 66 66 2e
> RSP: 0018:ffffc90003147c10 EFLAGS: 00010207
> RAX: dffffc0000000000 RBX: ffffffff8b5a8b4e RCX: 707d8ea8101f1b00
> RDX: 0000000000000000 RSI: ffffffff8b5a8b4e RDI: 0000000000000003
> RBP: ffffffff824e37fd R08: 0000000000000001 R09: 0000000000000000
> R10: dffffc0000000000 R11: fffffbfff1c0c6f3 R12: 0000000000000000
> R13: 000000000000001c R14: 000000000000001c R15: 0000000000000001
> FS:  000055556de07500(0000) GS:ffff888125f8b000(0000) knlGS:0000000000000000
> CS:  0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> CR2: 00007f09c332b5a0 CR3: 00000000750b0000 CR4: 00000000003526f0
> Call Trace:
>  <TASK>
>  __kasan_check_byte+0x12/0x40 mm/kasan/common.c:572
>  kasan_check_byte include/linux/kasan.h:401 [inline]
>  lock_acquire+0x84/0x340 kernel/locking/lockdep.c:5842
>  __raw_spin_lock include/linux/spinlock_api_smp.h:133 [inline]
>  _raw_spin_lock+0x2e/0x40 kernel/locking/spinlock.c:154
>  spin_lock include/linux/spinlock.h:351 [inline]
>  fsnotify_group_stop_queueing fs/notify/group.c:39 [inline]
>  fsnotify_destroy_group+0x8d/0x320 fs/notify/group.c:58
>  class_fsnotify_group_destructor fs/notify/fanotify/fanotify_user.c:1600 [inline]
>  __do_sys_fanotify_init fs/notify/fanotify/fanotify_user.c:1759 [inline]
>  __se_sys_fanotify_init+0x991/0xbc0 fs/notify/fanotify/fanotify_user.c:1607
>  do_syscall_x64 arch/x86/entry/syscall_64.c:63 [inline]
>  do_syscall_64+0xfa/0xf80 arch/x86/entry/syscall_64.c:94
>  entry_SYSCALL_64_after_hwframe+0x77/0x7f
> RIP: 0033:0x7f09c338f749
> Code: ff ff c3 66 2e 0f 1f 84 00 00 00 00 00 0f 1f 40 00 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 a8 ff ff ff f7 d8 64 89 01 48
> RSP: 002b:00007ffd8b6be3e8 EFLAGS: 00000246 ORIG_RAX: 000000000000012c
> RAX: ffffffffffffffda RBX: 00007f09c35e5fa0 RCX: 00007f09c338f749
> RDX: 0000000000000000 RSI: 0000000000000000 RDI: 0000000000000f00
> RBP: 00007ffd8b6be440 R08: 0000000000000000 R09: 0000000000000000
> R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000001
> R13: 00007f09c35e5fa0 R14: 00007f09c35e5fa0 R15: 0000000000000002
>  </TASK>
> Modules linked in:
> 
> Fixes: 3a6b564a6beb ("fanotify: convert fanotify_init() to FD_PREPARE()")
> Reported-by: syzbot+321168dfa622eda99689@...kaller.appspotmail.com
> Closes: https://lore.kernel.org/lkml/6928b121.a70a0220.d98e3.0110.GAE@google.com/
> Signed-off-by: Kuniyuki Iwashima <kuniyu@...gle.com>

Thanks for fixing this! The patch looks good to me. Feel free to add:

Reviewed-by: Jan Kara <jack@...e.cz>

								Honza

> ---
>  fs/notify/fanotify/fanotify_user.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c
> index be0a96ad4316..d0b9b984002f 100644
> --- a/fs/notify/fanotify/fanotify_user.c
> +++ b/fs/notify/fanotify/fanotify_user.c
> @@ -1598,10 +1598,10 @@ static struct hlist_head *fanotify_alloc_merge_hash(void)
>  }
>  
>  DEFINE_CLASS(fsnotify_group,
> -	      struct fsnotify_group *,
> -	      if (_T) fsnotify_destroy_group(_T),
> -	      fsnotify_alloc_group(ops, flags),
> -	      const struct fsnotify_ops *ops, int flags)
> +	     struct fsnotify_group *,
> +	     if (!IS_ERR_OR_NULL(_T)) fsnotify_destroy_group(_T),
> +	     fsnotify_alloc_group(ops, flags),
> +	     const struct fsnotify_ops *ops, int flags)
>  
>  /* fanotify syscalls */
>  SYSCALL_DEFINE2(fanotify_init, unsigned int, flags, unsigned int, event_f_flags)
> -- 
> 2.52.0.158.g65b55ccf14-goog
> 
-- 
Jan Kara <jack@...e.com>
SUSE Labs, CR