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

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

[Yu Kuai]

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>
---
Changes in v2:
 - don't call vfs_getattr_nosec() for block special file path, by Ming

 drivers/block/loop.c | 26 ++++++++++++++++----------
 1 file changed, 16 insertions(+), 10 deletions(-)

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 57263c273f0f..053a086d547e 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -139,20 +139,26 @@ static int part_shift;
 
 static loff_t lo_calculate_size(struct loop_device *lo, struct file *file)
 {
-	struct kstat stat;
 	loff_t loopsize;
 	int ret;
 
-	/*
-	 * Get the accurate file size. This provides better results than
-	 * cached inode data, particularly for network filesystems where
-	 * metadata may be stale.
-	 */
-	ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
-	if (ret)
-		return 0;
+	if (S_ISBLK(file_inode(file)->i_mode)) {
+		loopsize = i_size_read(file->f_mapping->host);
+	} else {
+		struct kstat stat;
+
+		/*
+		 * Get the accurate file size. This provides better results than
+		 * cached inode data, particularly for network filesystems where
+		 * metadata may be stale.
+		 */
+		ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
+		if (ret)
+			return 0;
+
+		loopsize = stat.size;
+	}
 
-	loopsize = stat.size;
 	if (lo->lo_offset > 0)
 		loopsize -= lo->lo_offset;
 	/* offset is beyond i_size, weird but possible */
-- 
2.39.2

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

[Christoph Hellwig]

On Mon, Aug 25, 2025 at 05:32:05PM +0800, Yu Kuai wrote:
> From: Yu Kuai <yukuai3@huawei.com>
> 
> By default, /dev/sda is block specail file from devtmpfs, getattr will

s/specail/special/

Otherwise looks good:

Reviewed-by: Christoph Hellwig <hch@lst.de>

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

[Ming Lei]

On Mon, Aug 25, 2025 at 5:40 PM Yu Kuai <yukuai1@huaweicloud.com> 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>
> ---
> Changes in v2:
>  - don't call vfs_getattr_nosec() for block special file path, by Ming

Reviewed-by: Ming Lei <ming.lei@redhat.com>

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

[Jens Axboe]

On Mon, 25 Aug 2025 17:32:05 +0800, Yu Kuai wrote:
> 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.
> 
> [...]

Applied, thanks!

[1/1] loop: fix zero sized loop for block special file
      commit: d14469ed7c00314fe8957b2841bda329e4eaf4ab

Best regards,
-- 
Jens Axboe

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

[Jens Axboe]

On 8/25/25 3:32 AM, Yu Kuai wrote:
>  static loff_t lo_calculate_size(struct loop_device *lo, struct file *file)
>  {
> -	struct kstat stat;
>  	loff_t loopsize;
>  	int ret;
>  
> -	/*
> -	 * Get the accurate file size. This provides better results than
> -	 * cached inode data, particularly for network filesystems where
> -	 * metadata may be stale.
> -	 */
> -	ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
> -	if (ret)
> -		return 0;
> +	if (S_ISBLK(file_inode(file)->i_mode)) {
> +		loopsize = i_size_read(file->f_mapping->host);
> +	} else {
> +		struct kstat stat;
> +
> +		/*
> +		 * Get the accurate file size. This provides better results than
> +		 * cached inode data, particularly for network filesystems where
> +		 * metadata may be stale.
> +		 */
> +		ret = vfs_getattr_nosec(&file->f_path, &stat, STATX_SIZE, 0);
> +		if (ret)
> +			return 0;
> +
> +		loopsize = stat.size;
> +	}

Gah, that was pretty silly...

-- 
Jens Axboe

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

[Cloud User]

Hi Jens, Kuai,

Thank you for your feedback, and apologies for the delay.

I will send the v5 patch in two parts as before.
The final commit will include the fix you suggested.
Below is the final git diff; the key change is the added ISBLK check.

Best regards,
Rajeev Mishra

diff --git a/drivers/block/loop.c b/drivers/block/loop.c
index 1b6ee91f8eb9..1e81620c0c72 100644
--- a/drivers/block/loop.c
+++ b/drivers/block/loop.c
@@ -137,20 +137,33 @@ static void loop_global_unlock(struct loop_device *lo, bool global)
 static int max_part;
 static int part_shift;
 
-static loff_t get_size(loff_t offset, loff_t sizelimit, struct file *file)
+static loff_t lo_calculate_size(struct loop_device *lo, struct file *file)
 {
+	struct kstat stat;
 	loff_t loopsize;
+	int ret;
 
-	/* Compute loopsize in bytes */
-	loopsize = i_size_read(file->f_mapping->host);
-	if (offset > 0)
-		loopsize -= offset;
+	/*
+	 * Get the accurate file size. This provides better results than
+	 * cached inode data, particularly for network filesystems where
+	 * metadata may be stale.
+	 */
+	if (S_ISBLK(file->f_inode->i_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;
+	}
+
+	if (lo->lo_offset > 0)
+		loopsize -= lo->lo_offset;
 	/* offset is beyond i_size, weird but possible */
 	if (loopsize < 0)
 		return 0;
-
-	if (sizelimit > 0 && sizelimit < loopsize)
-		loopsize = sizelimit;
+	if (lo->lo_sizelimit > 0 && lo->lo_sizelimit < loopsize)
+		loopsize = lo->lo_sizelimit;
 	/*
 	 * Unfortunately, if we want to do I/O on the device,
 	 * the number of 512-byte sectors has to fit into a sector_t.
@@ -158,11 +171,6 @@ static loff_t get_size(loff_t offset, loff_t sizelimit, struct file *file)
 	return loopsize >> 9;
 }
 
-static loff_t get_loop_size(struct loop_device *lo, struct file *file)
-{
-	return get_size(lo->lo_offset, lo->lo_sizelimit, file);
-}
-
 /*
  * We support direct I/O only if lo_offset is aligned with the logical I/O size
  * of backing device, and the logical block size of loop is bigger than that of
@@ -569,7 +577,7 @@ static int loop_change_fd(struct loop_device *lo, struct block_device *bdev,
 	error = -EINVAL;
 
 	/* size of the new backing store needs to be the same */
-	if (get_loop_size(lo, file) != get_loop_size(lo, old_file))
+	if (lo_calculate_size(lo, file) != lo_calculate_size(lo, old_file))
 		goto out_err;
 
 	/*
@@ -1063,7 +1071,7 @@ static int loop_configure(struct loop_device *lo, blk_mode_t mode,
 	loop_update_dio(lo);
 	loop_sysfs_init(lo);
 
-	size = get_loop_size(lo, file);
+	size = lo_calculate_size(lo, file);
 	loop_set_size(lo, size);
 
 	/* Order wrt reading lo_state in loop_validate_file(). */
@@ -1255,8 +1263,7 @@ loop_set_status(struct loop_device *lo, const struct loop_info64 *info)
 	if (partscan)
 		clear_bit(GD_SUPPRESS_PART_SCAN, &lo->lo_disk->state);
 	if (!err && size_changed) {
-		loff_t new_size = get_size(lo->lo_offset, lo->lo_sizelimit,
-					   lo->lo_backing_file);
+		loff_t new_size = lo_calculate_size(lo, lo->lo_backing_file);
 		loop_set_size(lo, new_size);
 	}
 out_unlock:
@@ -1399,7 +1406,7 @@ static int loop_set_capacity(struct loop_device *lo)
 	if (unlikely(lo->lo_state != Lo_bound))
 		return -ENXIO;
 
-	size = get_loop_size(lo, lo->lo_backing_file);
+	size = lo_calculate_size(lo, lo->lo_backing_file);
 	loop_set_size(lo, size);
 
 	return 0;