[PATCH v2] btrfs: reject refs=0 subvolume without DEAD in SUBVOL_SYNC_WAIT

[PATCH v2] btrfs: reject refs=0 subvolume without DEAD in SUBVOL_SYNC_WAIT

[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@vger.kernel.org
Suggested-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
Link: https://lore.kernel.org/linux-btrfs/dd97efe9-91ec-495b-b698-acff1447421c@gmx.com/
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
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@gmail.com/

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;
+		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
-- 
2.34.1

Re: [PATCH v2] btrfs: reject refs=0 subvolume without DEAD in SUBVOL_SYNC_WAIT

[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@vger.kernel.org
> Suggested-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
> Link: https://lore.kernel.org/linux-btrfs/dd97efe9-91ec-495b-b698-acff1447421c@gmx.com/
> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
> ---
> 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@gmail.com/
> 
> 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.


> +		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

Re: [PATCH v2] btrfs: reject refs=0 subvolume without DEAD in SUBVOL_SYNC_WAIT

[Qu Wenruo]

在 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@vger.kernel.org
>> Suggested-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
>> Link: https://lore.kernel.org/linux-btrfs/dd97efe9-91ec-495b-b698- 
>> acff1447421c@gmx.com/
>> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
>> ---
>> 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@gmail.com/
>>
>> 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
> 

Re: [PATCH v2] btrfs: reject refs=0 subvolume without DEAD in SUBVOL_SYNC_WAIT

[Cheng Yongkang]

Hi Qu,

Thanks for taking the time to list these.

To be upfront about where this came from: I work on automated bug
finding for the kernel, and this ASSERT was one of its findings; the
analysis and the patch were LLM-assisted. I sent a fix rather than a
bare report because I had the impression that reports without a fix
get little attention, but I'm not the right person to rework
SUBVOL_SYNC_WAIT and add the btrfs-progs support and test cases you
describe.

One data point on the return value: in my testing on for-next, waiting
for a subvolume that was being deleted returned -ENOENT once the
cleaner had removed it, with and without this patch, so the success
path is never reached.

Since the tool keeps finding issues like this, I'd like to ask how you
would prefer to receive them:

- Would a [BUG] report with a reproducer, the tree and config, the
  trace and a short analysis be more useful to you than a patch I
  can't take further?
- For low-priority cases like this one (a debug-only ASSERT in an
  ioctl with no real users), would you rather I batch them, or not
  send them at all?

If the ASSERT fix alone is still useful, I can send a v3 that only
replaces it with -EUCLEAN. Otherwise I'm fine with dropping the patch
and leaving this as a report.

Thanks,
Cheng Yongkang

Re: [PATCH v2] btrfs: reject refs=0 subvolume without DEAD in SUBVOL_SYNC_WAIT

[Qu Wenruo]

在 2026/11/1 18:03, Cheng Yongkang 写道:
> Hi Qu,
> 
> Thanks for taking the time to list these.
> 
> To be upfront about where this came from: I work on automated bug
> finding for the kernel, and this ASSERT was one of its findings; the
> analysis and the patch were LLM-assisted. I sent a fix rather than a
> bare report because I had the impression that reports without a fix
> get little attention, but I'm not the right person to rework
> SUBVOL_SYNC_WAIT and add the btrfs-progs support and test cases you
> describe.
> 
> One data point on the return value: in my testing on for-next, waiting
> for a subvolume that was being deleted returned -ENOENT once the
> cleaner had removed it, with and without this patch, so the success
> path is never reached.
> 
> Since the tool keeps finding issues like this, I'd like to ask how you
> would prefer to receive them:
> 
> - Would a [BUG] report with a reproducer, the tree and config, the
>    trace and a short analysis be more useful to you than a patch I
>    can't take further?

Yes, that would be preferred.

If you want, you can attach your LLM generated fix as a reference.
If it's really some simple fix, one of the co-developers will push your 
patch with your fix.

If not, we will address it with your reported-by tags.

> - For low-priority cases like this one (a debug-only ASSERT in an
>    ioctl with no real users), would you rather I batch them, or not
>    send them at all?

If you have hit more than 10 of such ASSERT()s, report them in a batch.
Otherwise one by one is fine.

> 
> If the ASSERT fix alone is still useful, I can send a v3 that only
> replaces it with -EUCLEAN.

I'll merge the v2 fix with modified commit message. You have no need to 
resend.

> Otherwise I'm fine with dropping the patch
> and leaving this as a report.
> 
> Thanks,
> Cheng Yongkang