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

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

[Ming Lei]

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

[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.

> 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

[Yu Kuai]

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