Syzbot reported a slab-use-after-free issue in f2fs_write_end_io(): [ 86.643336] BUG: KASAN: slab-use-after-free in f2fs_write_end_io+0x9b9/0xb60 [ 86.644120] Read of size 4 at addr ffff88804357d170 by task kworker/u4:4/45 ... [ 86.656543] Call Trace: ... [ 86.660351] f2fs_write_end_io+0x9b9/0xb60 ... [ 86.685123] Allocated by task 5484: ... [ 86.688325] f2fs_fill_super+0x8c/0x6ec0 ... [ 86.697685] Freed by task 5484: ... [ 86.702700] kfree+0x1c0/0x660 [ 86.703273] kill_f2fs_super+0x5b6/0x6c0
The problem is a race condition between the shutdown of the filesystem (kill_f2fs_super) and the asynchronous I/O completion handler (f2fs_write_end_io). When unmounting, kill_f2fs_super() frees the sbi structure. However, if there are pending checkpoint data (CP_DATA) writes, the f2fs_write_end_io() callback might still be running. In the original code, f2fs_write_end_io() accesses sbi->cp_wait after decrementing the page count. If the page count drops to zero, f2fs_wait_on_all_pages() in the unmount path returns, allowing kill_f2fs_super() to free sbi. If the callback then tries to wake up waiters on sbi->cp_wait, a UAF occurs. To fix this, I applied a two-step solution: 1. In kill_f2fs_super(), explicitly wait for all CP_DATA pages to obtain a count of zero using f2fs_wait_on_all_pages(). This ensures specific synchronization for these metadata writes. 2. In f2fs_write_end_io(), move the wake_up() call INSIDE the bio_for_each_folio_all() loop. This ensures that the wakeup (which signals completion to the waiter) happens before processing of the bio is effectively 'done' from the perspective of the waiter. More importantly, it removes any access to 'sbi' after the loop, eliminating the UAF window. Reported-by: [email protected] Closes: https://syzkaller.appspot.com/bug?extid=b4444e3c972a7a124187 Signed-off-by: Szymon Wilczek <[email protected]> --- fs/f2fs/data.c | 11 ++++++++--- fs/f2fs/super.c | 1 + 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c index c30e69392a62..5808d73c2598 100644 --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -318,10 +318,13 @@ static void f2fs_write_end_io(struct bio *bio) { struct f2fs_sb_info *sbi; struct folio_iter fi; + bool is_close; iostat_update_and_unbind_ctx(bio); sbi = bio->bi_private; + is_close = is_sbi_flag_set(sbi, SBI_IS_CLOSE); + if (time_to_inject(sbi, FAULT_WRITE_IO)) bio->bi_status = BLK_STS_IOERR; @@ -360,10 +363,12 @@ static void f2fs_write_end_io(struct bio *bio) f2fs_del_fsync_node_entry(sbi, folio); folio_clear_f2fs_gcing(folio); folio_end_writeback(folio); - } - if (!get_pages(sbi, F2FS_WB_CP_DATA) && + + if (!is_close && type == F2FS_WB_CP_DATA && + !get_pages(sbi, F2FS_WB_CP_DATA) && wq_has_sleeper(&sbi->cp_wait)) - wake_up(&sbi->cp_wait); + wake_up(&sbi->cp_wait); + } bio_put(bio); } diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index c4c225e09dc4..c9ee3fae1958 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -5454,6 +5454,7 @@ static void kill_f2fs_super(struct super_block *sb) kill_block_super(sb); /* Release block devices last, after fscrypt_destroy_keyring(). */ if (sbi) { + f2fs_wait_on_all_pages(sbi, F2FS_WB_CP_DATA); destroy_device_list(sbi); kfree(sbi); sb->s_fs_info = NULL; -- 2.52.0 _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
