On Mon, Aug 28, 2023 at 02:28:48PM +0200, Christian Brauner wrote: > > Maybe I didn't read the commit log carefully enough, but why do we > > need to call kill_super_notify before free_anon_bdev and any potential > > action in ->kill_sb after calling kill_anon_super here given that > > we already add a call to kill_super_notify after ->kill_sb? > > Yeah, the commit log explains this. We leave the superblock on fs_supers > past sb->kill_sb() and notify after device closure. For block based > filesystems that's the correct thing. They don't rely on sb->s_fs_info > and we need to ensure that all devices are closed. > > But for filesystems like kernfs that rely on get_keyed_super() they rely > on sb->s_fs_info to recycle sbs. sb->s_fs_info is currently always freed > in sb->kill_sb() > > kernfs_kill_sb() > -> kill_anon_super() > -> kfree(info) > > For such fses sb->s_fs_info is freed with the superblock still on > fs_supers which means we get a UAF when the sb is still found on the > list. So for such filesystems we need to remove and notify before > sb->s_fs_info is freed. That's done in kill_anon_super(). For such > filesystems the call in deactivate_locked_super() is a nop. Ok, so I did fail to parse the commit log. Looks good: Reviewed-by: Christoph Hellwig <hch@xxxxxx>
