[v1] loop: fix zero sized loop for block special file

[PATCH] loop: fix zero sized loop for block special file

Posted by Yu Kuai 1 month, 1 week ago

From: Yu Kuai <yukuai3@huawei.com>

By default, /dev/sda is block specail file from devtmpfs, getattr will
return file size as zero, causing loop failed for raw block device.

We can add bdev_statx() to return device size, however this may introduce
changes that are not acknowledged by user. Fix this problem by reverting
changes for block special file, file mapping host is set to bdev inode
while opening, and use i_size_read() directly to get device size.

Fixes: 47b71abd5846 ("loop: use vfs_getattr_nosec for accurate file size")
Reported-by: kernel test robot <oliver.sang@intel.com>
Closes: https://lore.kernel.org/oe-lkp/202508200409.b2459c02-lkp@intel.com
Signed-off-by: Yu Kuai <yukuai3@huawei.com>
---
 drivers/block/loop.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 57263c273f0f..cde235bd883c 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -153,6 +153,9 @@ static loff_t lo_calculate_size(struct loop_device *lo, struct file *file)
 		return 0;
 
 	loopsize = stat.size;
+	if (!loopsize && S_ISBLK(stat.mode))
+		loopsize = i_size_read(file->f_mapping->host);
+
 	if (lo->lo_offset > 0)
 		loopsize -= lo->lo_offset;
 	/* offset is beyond i_size, weird but possible */
-- 
2.39.2

Re: [PATCH] loop: fix zero sized loop for block special file

Posted by Ming Lei 1 month, 1 week ago

On Mon, Aug 25, 2025 at 02:23:00PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> By default, /dev/sda is block specail file from devtmpfs, getattr will
> return file size as zero, causing loop failed for raw block device.
> 
> We can add bdev_statx() to return device size, however this may introduce
> changes that are not acknowledged by user. Fix this problem by reverting
> changes for block special file, file mapping host is set to bdev inode
> while opening, and use i_size_read() directly to get device size.
> 
> Fixes: 47b71abd5846 ("loop: use vfs_getattr_nosec for accurate file size")
> Reported-by: kernel test robot <oliver.sang@intel.com>
> Closes: https://lore.kernel.org/oe-lkp/202508200409.b2459c02-lkp@intel.com
> Signed-off-by: Yu Kuai <yukuai3@huawei.com>
> ---
>  drivers/block/loop.c | 3 +++
>  1 file changed, 3 insertions(+)
> 
> diff --git a/drivers/block/loop.c b/drivers/block/loop.c
> index 57263c273f0f..cde235bd883c 100644
> --- a/drivers/block/loop.c
> +++ b/drivers/block/loop.c
> @@ -153,6 +153,9 @@ static loff_t lo_calculate_size(struct loop_device *lo, struct file *file)
>  		return 0;
>  
>  	loopsize = stat.size;
> +	if (!loopsize && S_ISBLK(stat.mode))
> +		loopsize = i_size_read(file->f_mapping->host);

`stat $BDEV_PATH` never works for getting bdev size, so it looks wrong
to call vfs_getattr_nosec() with bdev path for retrieving bdev's size.

So just wondering why not take the following more readable way?

	/* vfs_getattr() never works for retrieving bdev size */
	if (S_ISBLK(stat.mode)) {
		loopsize = i_size_read(file->f_mapping->host);
	} else {
          ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
          if (ret)
                  return 0;
          loopsize = stat.size;
	}

Also the above looks like how application reads file size in case of bdev
involved.


Thanks,
Ming

Re: [PATCH] loop: fix zero sized loop for block special file

Posted by Christoph Hellwig 1 month, 1 week ago

On Mon, Aug 25, 2025 at 04:56:55PM +0800, Ming Lei wrote:
> `stat $BDEV_PATH` never works for getting bdev size, so it looks wrong
> to call vfs_getattr_nosec() with bdev path for retrieving bdev's size.

Exactly.

> So just wondering why not take the following more readable way?
> 
> 	/* vfs_getattr() never works for retrieving bdev size */
> 	if (S_ISBLK(stat.mode)) {
> 		loopsize = i_size_read(file->f_mapping->host);
> 	} else {
>           ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
>           if (ret)
>                   return 0;
>           loopsize = stat.size;
> 	}
> 
> Also the above looks like how application reads file size in case of bdev
> involved.

That's not just more readable, but simply the way to go.  Maybe split
it into a helper for readability, though.

Re: [PATCH] loop: fix zero sized loop for block special file

Posted by Yu Kuai 1 month, 1 week ago

Hi,

在 2025/08/25 17:18, Christoph Hellwig 写道:
> On Mon, Aug 25, 2025 at 04:56:55PM +0800, Ming Lei wrote:
>> `stat $BDEV_PATH` never works for getting bdev size, so it looks wrong
>> to call vfs_getattr_nosec() with bdev path for retrieving bdev's size.
> 
> Exactly.
> 

Ok.
>> So just wondering why not take the following more readable way?
>>
>> 	/* vfs_getattr() never works for retrieving bdev size */
>> 	if (S_ISBLK(stat.mode)) {
>> 		loopsize = i_size_read(file->f_mapping->host);
>> 	} else {
>>            ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
>>            if (ret)
>>                    return 0;
>>            loopsize = stat.size;
>> 	}

Just we can't use stat.mode here, I'll replace it with:

S_ISBLK(file_inode(file)->i_mode)

Thanks,
Kuai

>>
>> Also the above looks like how application reads file size in case of bdev
>> involved.
> 
> That's not just more readable, but simply the way to go.  Maybe split
> it into a helper for readability, though.
> .
>