linux-block.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] loop: fix zero sized loop for block special file
@ 2025-08-25  6:23 Yu Kuai
  2025-08-25  8:56 ` Ming Lei
  0 siblings, 1 reply; 4+ messages in thread
From: Yu Kuai @ 2025-08-25  6:23 UTC (permalink / raw)
  To: ming.lei, axboe, rajeevm, yukuai3
  Cc: linux-block, linux-kernel, yukuai1, yi.zhang, yangerkun,
	johnny.chenyi

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


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] loop: fix zero sized loop for block special file
  2025-08-25  6:23 [PATCH] loop: fix zero sized loop for block special file Yu Kuai
@ 2025-08-25  8:56 ` Ming Lei
  2025-08-25  9:18   ` Christoph Hellwig
  0 siblings, 1 reply; 4+ messages in thread
From: Ming Lei @ 2025-08-25  8:56 UTC (permalink / raw)
  To: Yu Kuai
  Cc: axboe, rajeevm, yukuai3, linux-block, linux-kernel, yi.zhang,
	yangerkun, johnny.chenyi, linux-fsdevel

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] loop: fix zero sized loop for block special file
  2025-08-25  8:56 ` Ming Lei
@ 2025-08-25  9:18   ` Christoph Hellwig
  2025-08-25  9:36     ` Yu Kuai
  0 siblings, 1 reply; 4+ messages in thread
From: Christoph Hellwig @ 2025-08-25  9:18 UTC (permalink / raw)
  To: Ming Lei
  Cc: Yu Kuai, axboe, rajeevm, yukuai3, linux-block, linux-kernel,
	yi.zhang, yangerkun, johnny.chenyi, linux-fsdevel

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] loop: fix zero sized loop for block special file
  2025-08-25  9:18   ` Christoph Hellwig
@ 2025-08-25  9:36     ` Yu Kuai
  0 siblings, 0 replies; 4+ messages in thread
From: Yu Kuai @ 2025-08-25  9:36 UTC (permalink / raw)
  To: Christoph Hellwig, Ming Lei
  Cc: Yu Kuai, axboe, rajeevm, linux-block, linux-kernel, yi.zhang,
	yangerkun, johnny.chenyi, linux-fsdevel, yukuai (C)

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


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2025-08-25  9:36 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-08-25  6:23 [PATCH] loop: fix zero sized loop for block special file Yu Kuai
2025-08-25  8:56 ` Ming Lei
2025-08-25  9:18   ` Christoph Hellwig
2025-08-25  9:36     ` Yu Kuai

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).