[PATCH v2] f2fs: fix use-after-free in f2fs_write_end_io

Szymon Wilczek posted 1 patch 1 month, 1 week ago
There is a newer version of this series
fs/f2fs/data.c  | 11 ++++++++---
fs/f2fs/super.c |  2 ++
2 files changed, 10 insertions(+), 3 deletions(-)
[PATCH v2] f2fs: fix use-after-free in f2fs_write_end_io
Posted by Szymon Wilczek 1 month, 1 week ago
Syzbot reported a slab-use-after-free issue in f2fs_write_end_io():

BUG: KASAN: slab-use-after-free in f2fs_write_end_io+0x9b9/0xb60
Read of size 4 at addr ffff88804357d170 by task kworker/u4:4/45

The race condition occurs between the filesystem unmount path
(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 CP_DATA writes, the f2fs_write_end_io() callback might
still be running in softirq context and attempt to access sbi->cp_wait,
causing a use-after-free.

To fix this:

1. In f2fs_write_end_io(), check SBI_IS_CLOSE flag early and skip the
   wake_up() call if the filesystem is shutting down. Move the wake_up
   inside the loop for correct synchronization.

2. In kill_f2fs_super(), after f2fs_wait_on_all_pages() returns (meaning
   the page count is zero), call synchronize_rcu() before kfree(sbi).
   Since bio completion callbacks run in softirq context, which is an
   implicit RCU read-side critical section, synchronize_rcu() ensures
   all in-flight callbacks have completed before we free sbi.

The combination of these two changes eliminates the UAF window: the
is_close check provides fast-path optimization (skip wake_up when no
one is waiting), while synchronize_rcu() provides the hard guarantee
that no callback is accessing sbi when we free it.

Reported-by: syzbot+b4444e3c972a7a124187@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=b4444e3c972a7a124187
Signed-off-by: Szymon Wilczek <swilczek.lx@gmail.com>
---
v2: Add synchronize_rcu() to wait for softirq bio callbacks to complete,
    addressing the race condition pointed out by Chao Yu where sbi could
    be freed while f2fs_write_end_io() was still accessing sbi->cp_wait.
---
 fs/f2fs/data.c  | 11 ++++++++---
 fs/f2fs/super.c |  2 ++
 2 files changed, 10 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..924bc30d08b6 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -5454,6 +5454,8 @@ 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);
+		synchronize_rcu();
 		destroy_device_list(sbi);
 		kfree(sbi);
 		sb->s_fs_info = NULL;
-- 
2.52.0
Re: [PATCH v2] f2fs: fix use-after-free in f2fs_write_end_io
Posted by Chao Yu 1 month, 1 week ago
On 12/26/2025 10:20 PM, Szymon Wilczek wrote:
> Syzbot reported a slab-use-after-free issue in f2fs_write_end_io():
> 
> BUG: KASAN: slab-use-after-free in f2fs_write_end_io+0x9b9/0xb60
> Read of size 4 at addr ffff88804357d170 by task kworker/u4:4/45
> 
> The race condition occurs between the filesystem unmount path
> (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 CP_DATA writes, the f2fs_write_end_io() callback might
> still be running in softirq context and attempt to access sbi->cp_wait,
> causing a use-after-free.
> 
> To fix this:
> 
> 1. In f2fs_write_end_io(), check SBI_IS_CLOSE flag early and skip the
>     wake_up() call if the filesystem is shutting down. Move the wake_up
>     inside the loop for correct synchronization.
> 
> 2. In kill_f2fs_super(), after f2fs_wait_on_all_pages() returns (meaning
>     the page count is zero), call synchronize_rcu() before kfree(sbi).
>     Since bio completion callbacks run in softirq context, which is an
>     implicit RCU read-side critical section, synchronize_rcu() ensures
>     all in-flight callbacks have completed before we free sbi.
> 
> The combination of these two changes eliminates the UAF window: the
> is_close check provides fast-path optimization (skip wake_up when no
> one is waiting), while synchronize_rcu() provides the hard guarantee
> that no callback is accessing sbi when we free it.
> 
> Reported-by: syzbot+b4444e3c972a7a124187@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b4444e3c972a7a124187
> Signed-off-by: Szymon Wilczek <swilczek.lx@gmail.com>
> ---
> v2: Add synchronize_rcu() to wait for softirq bio callbacks to complete,
>      addressing the race condition pointed out by Chao Yu where sbi could
>      be freed while f2fs_write_end_io() was still accessing sbi->cp_wait.
> ---
>   fs/f2fs/data.c  | 11 ++++++++---
>   fs/f2fs/super.c |  2 ++
>   2 files changed, 10 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);
> +	}

Do we still need above change? due to below change may guarantee sbi
won't be released before soft-irq completion?

Thanks,

>   
>   	bio_put(bio);
>   }
> diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
> index c4c225e09dc4..924bc30d08b6 100644
> --- a/fs/f2fs/super.c
> +++ b/fs/f2fs/super.c
> @@ -5454,6 +5454,8 @@ 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);
> +		synchronize_rcu();
>   		destroy_device_list(sbi);
>   		kfree(sbi);
>   		sb->s_fs_info = NULL;