linux-kernel - Re: [syzbot] Re: [syzbot] [ext4?] KASAN: slab-use-after-free Read in fsnotify

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] [thread-next>] [day] [month] [year] [list]

Date: Tue, 16 Apr 2024 20:55:46 +0300
From: Amir Goldstein <amir73il@...il.com>
To: Jan Kara <jack@...e.cz>
Cc: Hillf Danton <hdanton@...a.com>, 
	syzbot <syzbot+5e3f9b2a67b45f16d4e6@...kaller.appspotmail.com>, 
	linux-fsdevel@...r.kernel.org, syzkaller-bugs@...glegroups.com, 
	linux-kernel@...r.kernel.org, Christian Brauner <brauner@...nel.org>
Subject: Re: [syzbot] Re: [syzbot] [ext4?] KASAN: slab-use-after-free Read in fsnotify

> > > > Maybe it is ok to let go of the optimization in fsnotify(), considering
> > > > that we now have stronger optimizations in the inline hooks and
> > > > in __fsnotify_parent()?
> > > >
> > > > I think that Hillf's patch is missing setting s_fsnotify_info to NULL?
> > > >
> > > >  @@ -101,8 +101,8 @@ void fsnotify_sb_delete(struct super_blo
> > > >          wait_var_event(fsnotify_sb_watched_objects(sb),
> > > >                         !atomic_long_read(fsnotify_sb_watched_objects(sb)));
> > > >          WARN_ON(fsnotify_sb_has_priority_watchers(sb, FSNOTIFY_PRIO_CONTENT));
> > > > +       WRITE_ONCE(sb->s_fsnotify_info, NULL);
> > > > +       synchronize_srcu(&fsnotify_mark_srcu);
> > > >          kfree(sbinfo);
> > > >  }
> > >
> > > So I had a look into this. Yes, something like this should work. We'll see
> > > whether synchronize_srcu() won't slow down umount too much. If someone will
> > > complain, we'll have to find a better solution.
> > >
> >
> > Actually, kfree_rcu(sbinfo) may be enough.
> > We do not actually access sbinfo during mark iteration and
> > event handling, we only access it to get to the sb connector.
> >
> > Something like the attached patch?
>
> Hum, thinking about this some more - what if we just freed sb_info from
> destroy_super_work()? By then we definitely are not getting fsnotify()
> calls for the superblock so all the problems are solved.
>

Considering that this is the solution for security_sb_free()
I don't see why not have fsnotify_sb_free().
I'll prepare a patch.

Thanks!
Amir.