[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOQ4uxhQbWQ4hzrG2opRmL0pH5NfWBrkk0PG3aWAF3FODVpbfg@mail.gmail.com>
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.
Powered by blists - more mailing lists