The UAF scenario described in v1/v2 does not look valid. The wakeup is already before folio_end_writeback(), and I do not see enough basis to claim that sbi->cp_wait can be freed at that point.
After your comments, v3-v7 drifted to a different issue: using the post-decrement count to avoid missing the zero transition when another F2FS_WB_CP_DATA writeback is submitted before the second get_pages() check. That is not the same issue as the v1/v2 UAF description. Please drop this patch. If you think the wakeup-latency issue is still worth fixing separately, I can send a new patch without the UAF wording, Fixes tag, or stable Cc. Thanks, On Sat, Jun 20, 2026 at 3:57 PM Chao Yu <[email protected]> wrote: > > On 6/18/26 18:05, Wenjie Qi wrote: > > f2fs_write_end_io() decrements the writeback page counter and then reads > > it again with get_pages() to decide whether the last F2FS_WB_CP_DATA > > completion should wake cp_wait. > > > > That second read can miss the zero transition as below: > > Looks comments of v7 patch is quite different from the one of v1 patch? > > Quoted from v1: > > "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." > > I may found something interesting: v7 codes try to fix UAF bug described in > v1 comment, however v7 comment tries to explain what v2 codes want to do. > > I suspect your LLM goes another direction after prompted w/ my comments on > patch v1? Let me know I'm wrong. :P > > Thanks, > > > > > checkpoint end_io A submitter B > > - f2fs_wait_on_all_pages > > - get_pages() > 0 > > - prepare_to_wait(cp_wait) > > - io_schedule_timeout > > - f2fs_write_end_io > > - dec_page_count > > : count 1 -> 0 > > - f2fs_submit_page_write > > - inc_page_count > > : count 0 -> 1 > > - get_pages() > 0 > > : skip wake_up(cp_wait) > > > > The checkpoint thread can then keep sleeping until > > DEFAULT_SCHEDULE_TIMEOUT, even though end_io A completed the old last > > F2FS_WB_CP_DATA page. > > > > Use the post-decrement value for F2FS_WB_CP_DATA completions so the wakeup > > decision is tied to this completion. Keep the existing dec_page_count() > > path for other writeback counters. > > > > Fixes: e234088758fc ("f2fs: avoid wait if IO end up when do_checkpoint for > > better performance") > > Fixes: ce2739e482bc ("f2fs: fix to avoid UAF in f2fs_write_end_io()") > > Cc: [email protected] > > Signed-off-by: Wenjie Qi <[email protected]> > > --- > > fs/f2fs/data.c | 12 +++++++----- > > fs/f2fs/f2fs.h | 6 ++++++ > > 2 files changed, 13 insertions(+), 5 deletions(-) > > > > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c > > index d83a21998ec2..2afdcd209d54 100644 > > --- a/fs/f2fs/data.c > > +++ b/fs/f2fs/data.c > > @@ -392,15 +392,17 @@ 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() > > */ > > - if (type == F2FS_WB_CP_DATA && !get_pages(sbi, type) && > > - wq_has_sleeper(&sbi->cp_wait)) > > - wake_up(&sbi->cp_wait); > > + if (type == F2FS_WB_CP_DATA) { > > + if (!dec_page_count_return(sbi, type) && > > + wq_has_sleeper(&sbi->cp_wait)) > > + wake_up(&sbi->cp_wait); > > + } else { > > + dec_page_count(sbi, type); > > + } > > > > folio_clear_f2fs_gcing(folio); > > folio_end_writeback(folio); > > diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h > > index 9f24287de4c3..db750cef371d 100644 > > --- a/fs/f2fs/f2fs.h > > +++ b/fs/f2fs/f2fs.h > > @@ -2776,6 +2776,12 @@ static inline void dec_page_count(struct > > f2fs_sb_info *sbi, int count_type) > > atomic_dec(&sbi->nr_pages[count_type]); > > } > > > > +static inline int dec_page_count_return(struct f2fs_sb_info *sbi, > > + int count_type) > > +{ > > + return atomic_dec_return(&sbi->nr_pages[count_type]); > > +} > > + > > static inline void inode_dec_dirty_pages(struct inode *inode) > > { > > if (!S_ISDIR(inode->i_mode) && !S_ISREG(inode->i_mode) && > > > > base-commit: c0b65f6129c7fbb526e921dd60261650f1b2bef9 > _______________________________________________ Linux-f2fs-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
