diff mbox series

btrfs: zoned: fix zone number to sector/physical calculation

Message ID 20210527014659.2222813-1-naohiro.aota@wdc.com (mailing list archive)
State New, archived
Headers show
Series btrfs: zoned: fix zone number to sector/physical calculation | expand

Commit Message

Naohiro Aota May 27, 2021, 1:46 a.m. UTC 
In btrfs_get_dev_zone_info(), we have "u32 sb_zone" and calculate "sector_t
sector" by shifting it. But, this "sector" is calculated in 32bit, leading
it to be 0 for the 2nd superblock copy.

Since zone number is u32, shifting it to sector (sector_t) or physical
address (u64) can easily trigger a missing cast bug like this.

This commit introduces helpers to convert zone number to sector/LBA, so we
won't fall into the same pitfall again.

Fixes: 12659251ca5d ("btrfs: implement log-structured superblock for ZONED mode")
Cc: stable@vger.kernel.org # 5.11+
Reported-by: Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
---
 fs/btrfs/zoned.c | 23 ++++++++++++++++++-----
 1 file changed, 18 insertions(+), 5 deletions(-)

Comments

Naohiro Aota May 27, 2021, 5:42 a.m. UTC | #1 
On Thu, May 27, 2021 at 10:46:59AM +0900, Naohiro Aota wrote:
> In btrfs_get_dev_zone_info(), we have "u32 sb_zone" and calculate "sector_t
> sector" by shifting it. But, this "sector" is calculated in 32bit, leading
> it to be 0 for the 2nd superblock copy.
> 
> Since zone number is u32, shifting it to sector (sector_t) or physical
> address (u64) can easily trigger a missing cast bug like this.
> 
> This commit introduces helpers to convert zone number to sector/LBA, so we
> won't fall into the same pitfall again.
> 
> Fixes: 12659251ca5d ("btrfs: implement log-structured superblock for ZONED mode")
> Cc: stable@vger.kernel.org # 5.11+
> Reported-by: Dmitry Fomichev <Dmitry.Fomichev@wdc.com>
> Signed-off-by: Naohiro Aota <naohiro.aota@wdc.com>
> ---
>  fs/btrfs/zoned.c | 23 ++++++++++++++++++-----
>  1 file changed, 18 insertions(+), 5 deletions(-)
> 
> diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
> index 5feb76bdfc06..a7f77f0a5a86 100644
> --- a/fs/btrfs/zoned.c
> +++ b/fs/btrfs/zoned.c
> @@ -153,6 +153,19 @@ static inline u32 sb_zone_number(int shift, int mirror)
>  	return (u32)zone;
>  }
>  
> +static inline sector_t zone_start_sector(u32 zone_number, struct block_device *bdev)
> +{
> +	sector_t zone_sectors = bdev_zone_sectors(bdev);
> +
> +	return (sector_t)zone_number << ilog2(zone_sectors);
> +}
> +
> +static inline u64 zone_start_physical(u32 zone_number,
> +			   struct btrfs_zoned_device_info *zone_info)
> +{
> +	return (u64)zone_number << zone_info->zone_size_shift;
> +}
> +

I just noticed an indentation fix missed the patch. I will resend
revised version.
diff mbox series

Patch

diff --git a/fs/btrfs/zoned.c b/fs/btrfs/zoned.c
index 5feb76bdfc06..a7f77f0a5a86 100644
--- a/fs/btrfs/zoned.c
+++ b/fs/btrfs/zoned.c
@@ -153,6 +153,19 @@  static inline u32 sb_zone_number(int shift, int mirror)
 	return (u32)zone;
 }
 
+static inline sector_t zone_start_sector(u32 zone_number, struct block_device *bdev)
+{
+	sector_t zone_sectors = bdev_zone_sectors(bdev);
+
+	return (sector_t)zone_number << ilog2(zone_sectors);
+}
+
+static inline u64 zone_start_physical(u32 zone_number,
+			   struct btrfs_zoned_device_info *zone_info)
+{
+	return (u64)zone_number << zone_info->zone_size_shift;
+}
+
 /*
  * Emulate blkdev_report_zones() for a non-zoned device. It slices up the block
  * device into static sized chunks and fake a conventional zone on each of
@@ -408,8 +421,7 @@  int btrfs_get_dev_zone_info(struct btrfs_device *device)
 		if (sb_zone + 1 >= zone_info->nr_zones)
 			continue;
 
-		sector = sb_zone << (zone_info->zone_size_shift - SECTOR_SHIFT);
-		ret = btrfs_get_dev_zones(device, sector << SECTOR_SHIFT,
+		ret = btrfs_get_dev_zones(device, zone_start_physical(sb_zone, zone_info),
 					  &zone_info->sb_zones[sb_pos],
 					  &nr_zones);
 		if (ret)
@@ -736,7 +748,7 @@  int btrfs_sb_log_location_bdev(struct block_device *bdev, int mirror, int rw,
 	if (sb_zone + 1 >= nr_zones)
 		return -ENOENT;
 
-	ret = blkdev_report_zones(bdev, sb_zone << zone_sectors_shift,
+	ret = blkdev_report_zones(bdev, zone_start_sector(sb_zone, bdev),
 				  BTRFS_NR_SB_LOG_ZONES, copy_zone_info_cb,
 				  zones);
 	if (ret < 0)
@@ -841,7 +853,7 @@  int btrfs_reset_sb_log_zones(struct block_device *bdev, int mirror)
 		return -ENOENT;
 
 	return blkdev_zone_mgmt(bdev, REQ_OP_ZONE_RESET,
-				sb_zone << zone_sectors_shift,
+				zone_start_sector(sb_zone, bdev),
 				zone_sectors * BTRFS_NR_SB_LOG_ZONES, GFP_NOFS);
 }
 
@@ -893,7 +905,8 @@  u64 btrfs_find_allocatable_zones(struct btrfs_device *device, u64 hole_start,
 			if (!(end <= sb_zone ||
 			      sb_zone + BTRFS_NR_SB_LOG_ZONES <= begin)) {
 				have_sb = true;
-				pos = ((u64)sb_zone + BTRFS_NR_SB_LOG_ZONES) << shift;
+				pos = zone_start_physical(sb_zone + BTRFS_NR_SB_LOG_ZONES,
+					       zinfo);
 				break;
 			}