Re: [PATCH v2] btrfs: reject refs=0 subvolume without DEAD in SUBVOL_SYNC_WAIT
From: Qu Wenruo
Date: Sun Nov 01 2026 - 02:18:39 EST
在 2026/11/1 17:21, Qu Wenruo 写道:
在 2026/11/1 16:55, Cheng Yongkang 写道:
btrfs_ioctl_subvol_sync() assumes that a cached root with zero
references has BTRFS_ROOT_SUBVOL_DEAD set, and asserts that before
waiting for the cleaner. Two states break the assumption:
- A corrupt ROOT_ITEM with zero references and a matching orphan item,
on a writable filesystem.
- A failed subvolume deletion. btrfs_delete_subvolume() zeroes the
root references, aborts the transaction, which forces the filesystem
read-only, and then clears DEAD again. No on-disk metadata is
corrupted in this case.
With CONFIG_BTRFS_ASSERT=y, both reach a BUG().
Before waiting for a deleted subvolume, return -EROFS if the
filesystem is read-only, since the cleaner does not drop dead roots
then. The check runs on every iteration, so it also stops a waiter
when the filesystem becomes read-only during the wait. Subvolume
deletion holds subvol_sem for writing across the abort and the DEAD
rollback, and each iteration takes it for reading before inspecting
the root, so the read-only state and the cleared flag are seen
together. On a writable filesystem, report a zero-reference root
without DEAD and return -EUCLEAN instead of asserting.
Subvolumes that were not deleted or do not exist still return -EEXIST
and -ENOENT, also on a read-only filesystem.
Tested on btrfs for-next with CONFIG_BTRFS_ASSERT=y. A deletion made
to fail by fault injection after the references are cleared hits the
assertion in SUBVOL_SYNC_WAIT without this change and gets -EROFS with
it. A waiter on a deleted subvolume now returns -EROFS when the
filesystem is remounted read-only, instead of waiting until it is
signalled.
Fixes: 6c83d153ed86 ("btrfs: add new ioctl to wait for cleaned subvolumes")
Cc: stable@xxxxxxxxxxxxxxx
Suggested-by: Qu Wenruo <quwenruo.btrfs@xxxxxxx>
Link: https://lore.kernel.org/linux-btrfs/dd97efe9-91ec-495b-b698- acff1447421c@xxxxxxx/
Signed-off-by: Cheng Yongkang <teel4res@xxxxxxxxx>
---
Changes in v2:
- Return -EROFS on a read-only filesystem before waiting for a deleted
subvolume (Qu).
- Use unlikely() for the inconsistent-root check (Qu).
- Describe the failed-delete case separately from corrupt metadata.
- Add the Fixes tag and Cc stable.
v1: https://lore.kernel.org/linux-btrfs/20261025180258.349175-1- teel4res@xxxxxxxxx/
Testing notes (btrfs for-next d1dca33127ef, KASAN):
- The fault-injection case reached the cleared-references state in 2
of 4 attempts on each kernel. The other attempts aborted earlier, and
both kernels returned -EEXIST.
- FOR_ONE, FOR_QUEUED, COUNT and PEEK return the same results with and
without this change for live, missing and deleted subvolumes, on
writable and read-only mounts.
- The -EUCLEAN branch was not exercised. A crafted image with a
zero-reference root and no DEAD flag hits the assertion without this
change, but remounting that image read-write stalls in a transaction
commit with or without this change, before the filesystem becomes
writable.
fs/btrfs/ioctl.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 9857d6a9d456..5a1f2edfd831 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5166,8 +5166,14 @@ static int btrfs_ioctl_subvol_sync(struct btrfs_fs_info *fs_info, void __user *a
return -ENOENT;
wait_for_deletion = true;
- ASSERT(root_flags & BTRFS_ROOT_SUBVOL_DEAD, "root_flags=0x%llx",
- root_flags);
+ if (unlikely(test_bit(BTRFS_FS_STATE_RO, &fs_info->fs_state)))
+ return -EROFS;
I'm not suggesting such an ad-hoc check, but some more comprehensive checks, like mnt_want_write_file().
As this one won't handle aborted transaction.
But it's not a good idea using that mnt_want_write_file() either, as that will block pm suspension/hibernation, especially considering the wait will only be handled by cleaner thread.
The default timeout for waking up cleaner is 30s, which is definitely not suitable for such calls.
If you are not sure what to do with the RW check and cleaner behavior, just don't put an adhoc solution into the update.
At least it won't be any worse.
With that said, there are more problems with the ioctl.
- No real user
No one is utilizing BTRFS_SUBVOL_SYNC* flags. Btrfs-progs itself
doesn't even bother it.
So all these fixes are very low priority, unless there are some wild
programs utilizing this.
But even btrfs-progs doesn't utilize them, the possibility are very
low.
- Incorrect return value
For that wait loop, if we waited for several rounds, then the
subvolume is properly removed. Then we just return -ENOENT, which is
definitely not the correct return code.
- Inconsistent radix tree index checks
There is a U32 radix tree check, but there is no such check for any
other operations like btrfs_insert_fs_root(), which can lead to
subvolid conflicts.
With that said, I'd say it's more meaningful to address those problems, and add a proper btrfs-progs support for it, along with test cases.
Sure, if you only want to fix the ASSERT(), that's fine.
But I would prefer to take a little more time to find out the real problems deeper behind the scene, other than dumping tokens to just grab some low hanging fruit.
+ if (unlikely(!(root_flags & BTRFS_ROOT_SUBVOL_DEAD))) {
+ btrfs_err(fs_info,
+ "subvol %llu has refs 0 without DEAD flag",
+ args.subvolid);
+ return -EUCLEAN;
+ }
sched_ret = schedule_timeout_interruptible(HZ);
/* Early wake up or error. */
if (sched_ret != 0)
base-commit: d1dca33127efacd86ac550aa86352f8ea9f29a5a