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 0CCE43E450E for ; Wed, 10 Jun 2026 11:12:52 +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=1781089975; cv=none; b=PWluu6N+EKJN8BizDcgP1RvxfbUSWA9LmZfAWuOj0sZbDB16Hz8vRGh3HMG6ehdT14ORwG0s12A/7Mg905ePFa5zTGJge7TRyOKXOrs0k/ca+q1sDoQhsEOln6lc64zzYK813fL9seLvMd/uJnWFj0Se19KGMjYlJEOGrnva5Qg= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1781089975; c=relaxed/simple; bh=qwpFR2QJWtIcOSPiAwSEivtR/sPSbuVNIXBhIX/FkPo=; h=Message-ID:Date:MIME-Version:Cc:Subject:To:References:From: In-Reply-To:Content-Type; b=bqX/FM/cd/mO9nHVLeqVSuYaJTFv4qazFM4Hn/6tdsMJGuS6m88tw64pqQal8POVDHzq/T0NOVcK5A1CdnCM5aMr+mBfU9F3vWceLtX9b1PvyYN4jrYh4dAAmcHRkjJg9k8+I7KGtYNmmEZZaZik0lxKoZrtsQ+7qYG516NCFsk= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=HuiTBgSc; 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="HuiTBgSc" Received: by smtp.kernel.org (Postfix) with ESMTPSA id 19E4D1F00893; Wed, 10 Jun 2026 11:12:50 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel.org; s=k20260515; t=1781089972; bh=RgGWx+heAj+7S+ELvNVejwrAVzsVIXQkb484Oqk79Lc=; h=Date:Cc:Subject:To:References:From:In-Reply-To; b=HuiTBgSckLZuG5vLXqcSTnQOyO9rjv42gGQv2M7UCIFSGD6t05eyUWLOEtOCyoELA I6F4NtYeh5jU7CUgDP/oeCCr6sg/uX24yIC1L5BdVGX4+WI/DVV2ARYSnH80/bmxDD +4DUZKYl8vq76knX0OoaSmgWZFWlilNh1Uj8t6n32tiEHI5RlF/8xksK3VVBYsvPO3 8+RdqT53g2Z7E6zG0n/EzXf9LLHxyvJutfFPu6eoxzPFepPncFfA+IDPzwThzqz18H 7Tlp8rupL3cwVD6pEwQk6/bU0ynkTRX/ICpyG5bazhW0hm4TrBdBA+7CmkqtVf6TQ3 LJ2bVKqNIdKGQ== Message-ID: <64e16f06-e7de-404f-9424-bedb9067d90f@kernel.org> Date: Wed, 10 Jun 2026 19:12:48 +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, linux-f2fs-devel@lists.sourceforge.net, linux-kernel@vger.kernel.org, qiwenjie@xiaomi.com, stable@kernel.org Subject: Re: [PATCH v2] f2fs: avoid cp_wait use-after-free in f2fs_write_end_io() To: Wenjie Qi , jaegeuk@kernel.org References: <20260526034439.1017521-1-qiwenjie@xiaomi.com> Content-Language: en-US From: Chao Yu In-Reply-To: <20260526034439.1017521-1-qiwenjie@xiaomi.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On 5/26/26 11:44, Wenjie Qi wrote: > f2fs_write_end_io() 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 > then continue through f2fs_put_super() and 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 > sbi accesses must happen before dec_page_count() can drop the > F2FS_WB_CP_DATA count to zero. > > Use atomic_dec_and_lock_irqsave() for F2FS_WB_CP_DATA completions so the > zero transition is serialized with cp_wait.lock. When the count reaches > zero, wake waiters while holding the same lock. > > In f2fs_wait_on_all_pages(), prepare the waiter and recheck the page count > while holding cp_wait.lock before sleeping. This keeps the wakeup visible > to waiters without requiring the end_io callback to access sbi after the > F2FS_WB_CP_DATA count has reached zero. It also avoids a missed wakeup that > would otherwise make the waiter sleep until DEFAULT_SCHEDULE_TIMEOUT. > > Fixes: ce2739e482bc ("f2fs: fix to avoid UAF in f2fs_write_end_io()") > Cc: stable@kernel.org > Signed-off-by: Wenjie Qi > --- > fs/f2fs/checkpoint.c | 20 ++++++++++++++++++-- > fs/f2fs/data.c | 25 +++++++++++++++++-------- > 2 files changed, 35 insertions(+), 10 deletions(-) > > diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c > index c00a6b6ebcbd..b16d2d30ec6a 100644 > --- a/fs/f2fs/checkpoint.c > +++ b/fs/f2fs/checkpoint.c > @@ -1497,24 +1497,40 @@ static void unblock_operations(struct f2fs_sb_info *sbi) > f2fs_unlock_all(sbi); > } > > +static bool f2fs_prepare_cp_wait(struct f2fs_sb_info *sbi, > + struct wait_queue_entry *wait, int type) > +{ > + unsigned long flags; > + bool wait_more; > + > + prepare_to_wait(&sbi->cp_wait, wait, TASK_UNINTERRUPTIBLE); > + spin_lock_irqsave(&sbi->cp_wait.lock, flags); > + wait_more = get_pages(sbi, type); > + spin_unlock_irqrestore(&sbi->cp_wait.lock, flags); > + > + return wait_more; > +} > + > void f2fs_wait_on_all_pages(struct f2fs_sb_info *sbi, int type) > { > DEFINE_WAIT(wait); > > for (;;) { > - if (!get_pages(sbi, type)) > + if (!f2fs_prepare_cp_wait(sbi, &wait, type)) > break; > > if (unlikely(f2fs_cp_error(sbi) && > !is_sbi_flag_set(sbi, SBI_IS_CLOSE))) > break; > + finish_wait(&sbi->cp_wait, &wait); > > 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); > + if (!f2fs_prepare_cp_wait(sbi, &wait, type)) > + break; > io_schedule_timeout(DEFAULT_SCHEDULE_TIMEOUT); > } > finish_wait(&sbi->cp_wait, &wait); > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > index d83a21998ec2..d92f0b70ba2f 100644 > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -392,15 +392,24 @@ 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); > + if (type == F2FS_WB_CP_DATA) { > + unsigned long flags; > > - /* > - * we should access sbi before 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)) > - wake_up(&sbi->cp_wait); > + /* > + * Hold cp_wait.lock across the zero transition and the > + * wakeup so f2fs_wait_on_all_pages() cannot miss it or > + * free sbi before this callback stops touching cp_wait. > + */ > + if (atomic_dec_and_lock_irqsave(&sbi->nr_pages[type], > + &sbi->cp_wait.lock, > + flags)) { if (atomic_dec_return(&sbi->nr_pages[type]) && wq_has_sleeper(&sbi->cp_wait)） wake_up(&sbi->cp_wait); Is it enough to solve the issue? Thanks, > + wake_up_locked(&sbi->cp_wait); > + spin_unlock_irqrestore(&sbi->cp_wait.lock, > + flags); > + } > + } else { > + dec_page_count(sbi, type); > + } > > folio_clear_f2fs_gcing(folio); > folio_end_writeback(folio);