lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <83a7a539-e7b5-4ff5-b3d9-c1e9320bf090@kernel.org>
Date: Fri, 26 Dec 2025 10:58:23 +0800
From: Chao Yu <chao@...nel.org>
To: Szymon Wilczek <swilczek.lx@...il.com>, jaegeuk@...nel.org
Cc: chao@...nel.org, linux-f2fs-devel@...ts.sourceforge.net,
 linux-kernel@...r.kernel.org,
 syzbot+b4444e3c972a7a124187@...kaller.appspotmail.com
Subject: Re: [PATCH] f2fs: fix use-after-free in f2fs_write_end_io

On 12/24/2025 12:28 AM, Szymon Wilczek wrote:
> 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: syzbot+b4444e3c972a7a124187@...kaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=b4444e3c972a7a124187
> Signed-off-by: Szymon Wilczek <swilczek.lx@...il.com>
> ---
>   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);

Seems this check may race w/ set_sbi_flag(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) &&

If F2FS_WB_CP_DATA count is zero, then sbi may be released in kill_f2fs_super(),
we should not allow to access sbi->cp_wait below, right?

Thanks,

>   				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;


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ