On Thu 08-04-21 22:38:08, Zhang Yi wrote: > On 2021/4/8 21:56, Jan Kara wrote: > > On Thu 08-04-21 19:36:18, Zhang Yi wrote: > >> There is a race between bdev_try_to_free_page() and > >> jbd2_journal_destroy() that could end up triggering a use after free > >> issue about journal. > >> > >> drop cache umount filesystem > >> bdev_try_to_free_page() > >> get journal > >> jbd2_journal_try_to_free_buffers() ext4_put_super() > >> kfree(journal) > >> access journal <-- lead to UAF > >> > >> The above race also could happens between the bdev_try_to_free_page() > >> and the error path of ext4_fill_super(). This patch avoid this race by > >> add rcu protection around accessing sbi->s_journal in > >> bdev_try_to_free_page() and destroy the journal after an rcu grace > >> period. > >> > >> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx> > > > > OK, I see the problem. But cannot the use-after-free happen even for the > > superblock itself (e.g., EXT4_SB(sb)->s_journal dereference)? I don't see > > anything preventing that as blkdev_releasepage() just shamelessly does: > > > > super = BDEV_I(page->mapping->host)->bdev.bd_super > > > Hi, Jan. > > I checked the superblock. In theory, the bdev_try_to_free_page() is invoked > with page locked, the umount process will wait the page unlock on > kill_block_super()->..->kill_bdev()->truncate_inode_pages_range() before free > superblock, so I guess the use-after-free problem couldn't happen in general. > But I think it's fragile and may invalidate if the bdev has more than one > operners(__blkdev_put() call kill_bdev only if bd_openers becomes zero)? Yes, kill_bdev() is only called when bd_openers drops to 0 but there can be other processes having the bdev open (non-exclusively). Honza -- Jan Kara <jack@xxxxxxxx> SUSE Labs, CR
