From: Boris Burkov Date: Thu, 30 Jul 2026 16:38:02 +0000 (-0700) Subject: btrfs: flush the fixup workers during close_ctree X-Git-Url: http://git.ipfire.org/gitweb/?a=commitdiff_plain;h=ae2567b11c3df43861d05f856bdb3434b3961aa1;p=thirdparty%2Fkernel%2Flinux.git btrfs: flush the fixup workers during close_ctree Reintroducing the COW fixup worker brought back the unmount race fixed by commit 41fd1e94066a ("btrfs: wait for fixup workers before stopping cleaner kthread during umount") without bringing back the fix. A fixup work item queued by the final writeback pass can still be in flight when close_ctree() stops the cleaner kthread and frees the fs roots. While destroy_workqueue() drains the queue, that happens after the cleaner thread was freed, so btrfs_add_delayed_iput() called from the fixup worker is no longer safe (not to mention that we are already in BTRFS_FS_STATE_NO_DELAYED_IPUT when it runs). Therefore we need to bring back explicitly flushing the fixup workqueue as in Filipe's original fix. The first flush will catch all the fixup writeback queued during the final sync before umount, but some of that might hit memory allocation errors and stay fixup in the blocks/folio, leading any subsequent writeback triggered *inside* umount (e.g. reclaim workers shutting down) to hit it and queue again. To fix that, and the possibility of any really long-lived pinned folios getting marked, deny queueing new fixup during umount. That allows us to flush twice (once before doing a real writeback pass to get the actual data, second time to clean up any rather unlikely stragglers right before declaring BTRFS_FS_STATE_NO_DELAYED_IPUT) and be certain nothing got re-queued. Reproduced by injecting a one-shot 30s sleep at the head of btrfs_writepage_fixup_worker() on a KASAN kernel, running the normal reproducing read dio workload before unmount and then observing: BUG: KASAN: slab-use-after-free in _raw_spin_lock_irqsave+0x35/0x50 Read of size 1 at addr ffff88810b4b08f8 by task kworker/u32:5/219 Workqueue: btrfs-fixup btrfs_writepage_fixup_worker [btrfs] Call Trace: _raw_spin_lock_irqsave+0x35/0x50 try_to_wake_up+0xc0/0x18c0 btrfs_writepage_fixup_worker+0x7f3/0xf20 [btrfs] ... Fixes: 4be9c7da6860 ("btrfs: trigger cow fixup via dirty_folio()") Assisted-by: LLM (reproduction, analysis) Reviewed-by: Qu Wenruo Signed-off-by: Boris Burkov Signed-off-by: David Sterba --- diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c index 6bb70c43a63f..8bdc94d3ddee 100644 --- a/fs/btrfs/disk-io.c +++ b/fs/btrfs/disk-io.c @@ -4361,6 +4361,18 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info) /* clear out the rbtree of defraggable inodes */ btrfs_cleanup_defrag_inodes(fs_info); + /* + * Before the unmount, we sync down all the writeback which can + * generate fixup work. We are about to run delalloc for autodefrag so + * piggy back on that by also flushing the fixup work which can also + * generate delalloc we would like to get run. + * + * After this, it is still possible that some thread doing writeback is + * in btrfs_queue_writepage_fixup() and might finish queueing some final + * work, racing the btrfs_fs_closing() check there. + */ + flush_workqueue(fs_info->fixup_workers); + /* * Handle the error fs first, as it will flush and wait for all ordered * extents. This will generate delayed iputs, thus we want to handle @@ -4438,6 +4450,15 @@ void __cold close_ctree(struct btrfs_fs_info *fs_info) cancel_work_sync(&fs_info->preempt_reclaim_work); cancel_work_sync(&fs_info->em_shrinker_work); + /* + * Reclaim workers can run writeback which can queue fixup. + * After the above cancel_work_sync() calls, any such queueing attempts are + * guaranteed to see btrfs_fs_closing(), so at this point we can genuinely fully + * flush the fixup workqueue. This relies on the belief that *now* no thread can + * still be sitting in btrfs_queue_writepage_fixup(). + */ + flush_workqueue(fs_info->fixup_workers); + /* * Run delayed iputs again because an async reclaim worker may have * added new ones if it was flushing delalloc: diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c index 98b31a090626..9b1bf2e03497 100644 --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -2952,6 +2952,23 @@ void btrfs_queue_writepage_fixup(struct btrfs_inode *inode, struct folio *folio) struct btrfs_fs_info *fs_info = inode->root->fs_info; struct btrfs_writepage_fixup *fixup; + /* + * Disallow queueing more fixup during unmount to break the cycle + * of writeback queuing fixup queuing writeback etc. + * + * If it actually hit, then something which was fixup wasn't written + * which we should warn about. + */ + if (btrfs_fs_closing(fs_info)) { + btrfs_warn_rl(fs_info, + "dropping unqueued fixup blocks at unmount. root %lld ino %llu folio %llu", + btrfs_root_id(inode->root), btrfs_ino(inode), + folio_pos(folio)); + btrfs_folio_clear_fixup_dirty(fs_info, folio, + folio_pos(folio), folio_size(folio)); + return; + } + fixup = kzalloc_obj(*fixup, GFP_NOFS); if (!fixup) return;