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

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

[Cheng Yongkang]

A crafted image can leave a tree-root ROOT_ITEM with refs==0 and without
BTRFS_ROOT_SUBVOL_DEAD, plus a matching orphan item.  Mount accepts it,
and btrfs_ioctl_subvol_sync() then reaches

  ASSERT(root_flags & BTRFS_ROOT_SUBVOL_DEAD, ...)

which is BUG() on CONFIG_BTRFS_ASSERT=y.

Return -EUCLEAN instead of asserting, and log the inconsistent state.

The ioctl BUG was reproduced in 2/4 baseline boots (the other two lost
a race to the same image's unrelated cleaner WARN).  With this change
the ioctl returns -EUCLEAN and no BUG; a normal btrfs mount/write/
readback/check regression passes.

Assisted-by: LLM
Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
---
 fs/btrfs/ioctl.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
index 576cd76..00eab0b 100644
--- a/fs/btrfs/ioctl.c
+++ b/fs/btrfs/ioctl.c
@@ -5149,8 +5149,12 @@ 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 (!(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)
-- 
2.34.1

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

[Qu Wenruo]

在 2026/10/26 04:32, Cheng Yongkang 写道:
> A crafted image can leave a tree-root ROOT_ITEM with refs==0 and without
> BTRFS_ROOT_SUBVOL_DEAD, plus a matching orphan item.  Mount accepts it,
> and btrfs_ioctl_subvol_sync() then reaches
> 
>    ASSERT(root_flags & BTRFS_ROOT_SUBVOL_DEAD, ...)
> 
> which is BUG() on CONFIG_BTRFS_ASSERT=y.
> 
> Return -EUCLEAN instead of asserting, and log the inconsistent state.
> 
> The ioctl BUG was reproduced in 2/4 baseline boots (the other two lost
> a race to the same image's unrelated cleaner WARN).  With this change
> the ioctl returns -EUCLEAN and no BUG; a normal btrfs mount/write/
> readback/check regression passes.
> 
> Assisted-by: LLM
> Signed-off-by: Cheng Yongkang <teel4res@gmail.com>
> ---
>   fs/btrfs/ioctl.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c
> index 576cd76..00eab0b 100644
> --- a/fs/btrfs/ioctl.c
> +++ b/fs/btrfs/ioctl.c
> @@ -5149,8 +5149,12 @@ 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 (!(root_flags & BTRFS_ROOT_SUBVOL_DEAD)) {

Use unlikely().


Secondly it's still possible to hit a subvolume without DEAD flag at 
runtime.

btrfs_ioctl_snap_destroy()
|- btrfs_delete_subvolume()
    |- btrfs_set_root_flags( | SUBVOL_DEAD)
    |- Some error hit, goto out_end_trans or out_undead tag
    |- btrfs_set_root_flags( & ~SUBVOL_DEAD)

Although in that case, the transaction is already aborted, so no on-disk 
metadata is corrupted.

Then someone can still call the ioctl on the subvolume and trigger the 
warning.

Although this is not a regression, but a pre-existing problem, I'd 
suggest to do some writeable checks for the SYNC_WAIT ioctl.
> +			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)