From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-alma10-1.taild15c8.ts.net [100.103.45.18]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id C6F2E3815EC for ; Mon, 25 May 2026 12:15:11 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=100.103.45.18 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779711313; cv=none; b=DJD8URSh8Wf9X6ujLXOkkd5+k2QA65aBNh+YrwM34XbUNO/snm1HKnYHYwXyjrsGZyBbN5SrAiBEleHDxlFcg3phISCjWLsP0C8l9mGrcj3CxUuVGu3j3bDk/w7LKqBpePzMCGRfdjhjjCYGouySO9DioxA8eqh5M03zCTlTqqI= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1779711313; c=relaxed/simple; bh=VDcXK2fZ2PX14AKEKAKnfnGiWm2nn2qFez0lBiXb9K4=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=VWjmJ40Ey6F8dqm2URyvuTwPGmQ1O7WIikUuaoTkeWh9KMvlfVswN6OVi5XdKrKOjAiIxiWYw/2RXFjy6UsX7Nol9qPMsCxmCvzotfW4mBiGbU9RHy8VW+kP4+TDsUfozszV94ew8ly0+norbKC6+FhCYQxQ/QEEI+SppfoqHkc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=F+BYWNDJ; arc=none smtp.client-ip=100.103.45.18 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="F+BYWNDJ" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 88BCC1F000E9; Mon, 25 May 2026 12:15:09 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1779711311; bh=YwmYeBFlqqwtunb0YVMfJONsz3dZjRTkfJZWi8iMnMw=; h=Date:Cc:Subject:To:References:From:In-Reply-To; b=F+BYWNDJvexcMY4vpQ20WgvP6BSLXbGlElchCA4fjZ8TPmc/SOzoluHEum107N8+n g8TKf3tqqS4RFJW04LQlvVbGoOBGg5bevJEDTgbJ5lfuabw6GnDlcwJppNGWrUq/62 9q2QGfLnGmEmRovowXTKaSGERu7BcD03nepm4b+8apur9MUD0NnLe6wutO9+vDuWe5 ImtVxVWNFdi5fVwf9VlvbNhQIapqHRl7Nru3WVanPzOsfajJnPIvF8JlqLSjTmBXkQ wx3HRGHySjyzovqRerAsNeehQdO55MTP08UcSWuXjskKFTzK895f/zBa0BfvVGg5uF t7gOAsXSukE7A== Message-ID: <6a5068fe-b62b-4f43-b0ae-ab1c7f70e078@kernel.org> Date: Mon, 25 May 2026 20:15:06 +0800 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Cc: chao@kernel.org, yangyongpeng@xiaomi.com, geoo115@gmail.com, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, qiwenjie@xiaomi.com, stable@kernel.org Subject: Re: [PATCH] f2fs: avoid cp_wait use-after-free in f2fs_write_end_io() To: Wenjie Qi , jaegeuk@kernel.org References: <20260525053016.169150-1-qiwenjie@xiaomi.com> Content-Language: en-US From: Chao Yu In-Reply-To: <20260525053016.169150-1-qiwenjie@xiaomi.com> Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit On 5/25/26 13:30, Wenjie Qi wrote: > f2fs_write_end_io() currently decrements the writeback page counter before > waking sbi->cp_wait for the last F2FS_WB_CP_DATA completion. > > That decrement can drop the F2FS_WB_CP_DATA count to zero. It can unblock > a concurrent unmount path waiting in f2fs_wait_on_all_pages(). Unmount can > continue through f2fs_put_super() and eventually free sbi while the end_io > callback is still about to evaluate wq_has_sleeper() and wake_up() on > sbi->cp_wait. > > Commit 2d9c4a4ed4ee ("f2fs: fix UAF caused by decrementing sbi->nr_pages[] > in f2fs_write_end_io()") fixed one post-decrement sbi access by moving the > warm-node-list handling before dec_page_count(). The compressed writeback > path follows the same rule and documents that dec_page_count() must be the > last access to sbi when it can drop F2FS_WB_CP_DATA to zero. > > Apply the same ordering rule to the cp_wait wakeup. Check whether this is > the last F2FS_WB_CP_DATA completion and wake the waiter before the counter > decrement. Then the callback no longer dereferences sbi->cp_wait after the > lifetime boundary. A waiter that runs before the decrement may observe old > count and sleep until the one-jiffy timeout, but correctness no longer > depends on touching sbi after the counter reaches zero. > > Fixes: ce2739e482bc ("f2fs: fix to avoid UAF in f2fs_write_end_io()") > Cc: stable@kernel.org > Signed-off-by: Wenjie Qi > --- > fs/f2fs/data.c | 12 ++++++------ > 1 file changed, 6 insertions(+), 6 deletions(-) > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index d83a21998ec2..b1e9fb5ca159 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -392,16 +392,16 @@ static void f2fs_write_end_io(struct bio *bio) > if (f2fs_in_warm_node_list(folio)) > f2fs_del_fsync_node_entry(sbi, folio); > > - dec_page_count(sbi, type); > - > /* > - * we should access sbi before folio_end_writeback() to > - * avoid racing w/ kill_f2fs_super() > + * Access sbi before dec_page_count() and folio_end_writeback() > + * to avoid racing w/ kill_f2fs_super(). > */ > - if (type == F2FS_WB_CP_DATA && !get_pages(sbi, type) && > - wq_has_sleeper(&sbi->cp_wait)) > + if (type == F2FS_WB_CP_DATA && get_pages(sbi, type) == 1 && > + wq_has_sleeper(&sbi->cp_wait)) > wake_up(&sbi->cp_wait); If we call dec_page_count() after wake_up(), get_pages() in below function may return true, and then ckpt thread may need to wait on cp_wait for another DEFAULT_SCHEDULE_TIMEOUT? void f2fs_wait_on_all_pages(struct f2fs_sb_info *sbi, int type) { DEFINE_WAIT(wait); for (;;) { if (!get_pages(sbi, type)) break; if (unlikely(f2fs_cp_error(sbi) && !is_sbi_flag_set(sbi, SBI_IS_CLOSE))) break; if (type == F2FS_DIRTY_META) f2fs_sync_meta_pages(sbi, LONG_MAX, FS_CP_META_IO); else if (type == F2FS_WB_CP_DATA) f2fs_submit_merged_write(sbi, DATA); prepare_to_wait(&sbi->cp_wait, &wait, TASK_UNINTERRUPTIBLE); io_schedule_timeout(DEFAULT_SCHEDULE_TIMEOUT); } finish_wait(&sbi->cp_wait, &wait); } How about: static inline int dec_return_page_count(struct f2fs_sb_info *sbi, int count_type) { return atomic_dec_return(&sbi->nr_pages[count_type]); } f2fs_write_end_io() { ... bool need_wakeup = false; ... if (type == F2FS_WB_CP_DATA) need_wakeup = !dec_return_page_count(sbi, type); else dec_page_count(sbi, type); if (need_wakeup && wq_has_sleeper(&sbi->cp_wait)) wake_up(&sbi->cp_wait); ... } Thanks, > > + dec_page_count(sbi, type); > + > folio_clear_f2fs_gcing(folio); > folio_end_writeback(folio); > }