[PATCH] btrfs: don't allow adding block device with 0 bytes

[PATCH] btrfs: don't allow adding block device with 0 bytes

[Mark Harmstone]

Commit 15ae0410 in btrfs-progs inadvertently changed it so that if the
BLKGETSIZE64 ioctl on a block device returned a size of 0, this was no
longer seen as an error condition.

Unfortunately this is how disconnected NBD devices behave, meaning that
with btrfs-progs 6.16 it's now possible to add a device you can't
remove:

~ # btrfs device add /dev/nbd0 /root/temp
~ # btrfs device remove /dev/nbd0 /root/temp
ERROR: error removing device '/dev/nbd0': Invalid argument

This check should always have been done kernel-side anyway, so add a
check in btrfs_init_new_device() that the new device doesn't have a size
of 0.

Signed-off-by: Mark Harmstone <mark@harmstone.com>
---
 fs/btrfs/volumes.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
index 63b65f70a2b3..0757a546ff5c 100644
--- a/fs/btrfs/volumes.c
+++ b/fs/btrfs/volumes.c
@@ -2726,6 +2726,11 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
 		goto error;
 	}
 
+	if (bdev_nr_bytes(file_bdev(bdev_file)) == 0) {
+		ret = -EINVAL;
+		goto error;
+	}
+
 	if (fs_devices->seeding) {
 		seeding_dev = true;
 		down_write(&sb->s_umount);
-- 
2.49.1

Re: [PATCH] btrfs: don't allow adding block device with 0 bytes

[Qu Wenruo]

在 2025/8/27 20:07, Mark Harmstone 写道:
> Commit 15ae0410 in btrfs-progs inadvertently changed it so that if the
> BLKGETSIZE64 ioctl on a block device returned a size of 0, this was no
> longer seen as an error condition.

My bad, that commit changed the handling of zero sized device, 
previously the caller who checks the returned size now only checks if 
the ioctl/stat works.

Feel free to send out a progs fix.
> 
> Unfortunately this is how disconnected NBD devices behave, meaning that
> with btrfs-progs 6.16 it's now possible to add a device you can't
> remove:
> 
> ~ # btrfs device add /dev/nbd0 /root/temp
> ~ # btrfs device remove /dev/nbd0 /root/temp
> ERROR: error removing device '/dev/nbd0': Invalid argument
> 
> This check should always have been done kernel-side anyway, so add a
> check in btrfs_init_new_device() that the new device doesn't have a size
> of 0.
> 
> Signed-off-by: Mark Harmstone <mark@harmstone.com>

The extra kernel checks looks good to me.

Reviewed-by: Qu Wenruo <wqu@suse.com>

Thanks,
Qu

> ---
>   fs/btrfs/volumes.c | 5 +++++
>   1 file changed, 5 insertions(+)
> 
> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
> index 63b65f70a2b3..0757a546ff5c 100644
> --- a/fs/btrfs/volumes.c
> +++ b/fs/btrfs/volumes.c
> @@ -2726,6 +2726,11 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>   		goto error;
>   	}
>   
> +	if (bdev_nr_bytes(file_bdev(bdev_file)) == 0) {
> +		ret = -EINVAL;
> +		goto error;
> +	}
> +
>   	if (fs_devices->seeding) {
>   		seeding_dev = true;
>   		down_write(&sb->s_umount);

Re: [PATCH] btrfs: don't allow adding block device with 0 bytes

[Qu Wenruo]

在 2025/8/27 20:14, Qu Wenruo 写道:
> 
> 
> 在 2025/8/27 20:07, Mark Harmstone 写道:
>> Commit 15ae0410 in btrfs-progs inadvertently changed it so that if the
>> BLKGETSIZE64 ioctl on a block device returned a size of 0, this was no
>> longer seen as an error condition.
> 
> My bad, that commit changed the handling of zero sized device, 
> previously the caller who checks the returned size now only checks if 
> the ioctl/stat works.
> 
> Feel free to send out a progs fix.
>>
>> Unfortunately this is how disconnected NBD devices behave, meaning that
>> with btrfs-progs 6.16 it's now possible to add a device you can't
>> remove:
>>
>> ~ # btrfs device add /dev/nbd0 /root/temp
>> ~ # btrfs device remove /dev/nbd0 /root/temp
>> ERROR: error removing device '/dev/nbd0': Invalid argument
>>
>> This check should always have been done kernel-side anyway, so add a
>> check in btrfs_init_new_device() that the new device doesn't have a size
>> of 0.
>>
>> Signed-off-by: Mark Harmstone <mark@harmstone.com>
> 
> The extra kernel checks looks good to me.
> 
> Reviewed-by: Qu Wenruo <wqu@suse.com>

In fact this reminds me to try other extreme situations.

Like adding a 64K sized device.

The result is, the fs will never be able to be mounted again, as we 
saved the cursed device which can not have the primary super block written.

Since we're here, can we enlarge the bdev size to <= 
BTRFS_DEVICE_RANGE_RESERVED?

Thanks,
Qu
> 
> Thanks,
> Qu
> 
>> ---
>>   fs/btrfs/volumes.c | 5 +++++
>>   1 file changed, 5 insertions(+)
>>
>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>> index 63b65f70a2b3..0757a546ff5c 100644
>> --- a/fs/btrfs/volumes.c
>> +++ b/fs/btrfs/volumes.c
>> @@ -2726,6 +2726,11 @@ int btrfs_init_new_device(struct btrfs_fs_info 
>> *fs_info, const char *device_path
>>           goto error;
>>       }
>> +    if (bdev_nr_bytes(file_bdev(bdev_file)) == 0) {
>> +        ret = -EINVAL;
>> +        goto error;
>> +    }
>> +
>>       if (fs_devices->seeding) {
>>           seeding_dev = true;
>>           down_write(&sb->s_umount);
> 

Re: [PATCH] btrfs: don't allow adding block device with 0 bytes

[Mark Harmstone]

On 27/08/2025 11.05 pm, Qu Wenruo wrote:
> 
> 
> 在 2025/8/27 20:14, Qu Wenruo 写道:
>>
>>
>> 在 2025/8/27 20:07, Mark Harmstone 写道:
>>> Commit 15ae0410 in btrfs-progs inadvertently changed it so that if the
>>> BLKGETSIZE64 ioctl on a block device returned a size of 0, this was no
>>> longer seen as an error condition.
>>
>> My bad, that commit changed the handling of zero sized device, previously the caller who checks the returned size now only checks if the ioctl/stat works.
>>
>> Feel free to send out a progs fix.
>>>
>>> Unfortunately this is how disconnected NBD devices behave, meaning that
>>> with btrfs-progs 6.16 it's now possible to add a device you can't
>>> remove:
>>>
>>> ~ # btrfs device add /dev/nbd0 /root/temp
>>> ~ # btrfs device remove /dev/nbd0 /root/temp
>>> ERROR: error removing device '/dev/nbd0': Invalid argument
>>>
>>> This check should always have been done kernel-side anyway, so add a
>>> check in btrfs_init_new_device() that the new device doesn't have a size
>>> of 0.
>>>
>>> Signed-off-by: Mark Harmstone <mark@harmstone.com>
>>
>> The extra kernel checks looks good to me.
>>
>> Reviewed-by: Qu Wenruo <wqu@suse.com>
> 
> In fact this reminds me to try other extreme situations.
> 
> Like adding a 64K sized device.
> 
> The result is, the fs will never be able to be mounted again, as we saved the cursed device which can not have the primary super block written.
> 
> Since we're here, can we enlarge the bdev size to <= BTRFS_DEVICE_RANGE_RESERVED?

Makes sense to me. I'll send another patch.

> 
> Thanks,
> Qu
>>
>> Thanks,
>> Qu
>>
>>> ---
>>>   fs/btrfs/volumes.c | 5 +++++
>>>   1 file changed, 5 insertions(+)
>>>
>>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c
>>> index 63b65f70a2b3..0757a546ff5c 100644
>>> --- a/fs/btrfs/volumes.c
>>> +++ b/fs/btrfs/volumes.c
>>> @@ -2726,6 +2726,11 @@ int btrfs_init_new_device(struct btrfs_fs_info *fs_info, const char *device_path
>>>           goto error;
>>>       }
>>> +    if (bdev_nr_bytes(file_bdev(bdev_file)) == 0) {
>>> +        ret = -EINVAL;
>>> +        goto error;
>>> +    }
>>> +
>>>       if (fs_devices->seeding) {
>>>           seeding_dev = true;
>>>           down_write(&sb->s_umount);
>>
>