[PATCH v2 0/2] block: make some queue limits checks more robust

[PATCH v2 0/2] block: make some queue limits checks more robust

[John Garry]

This series contains a couple of changes to make request queue limits
checks more robust.

About the change to enforce a power-of-2 physical block size, I audited
the drivers which set this. All look ok to comply with this rule, as
follows:

drivers/block/brd.c - uses PAGE_SIZE, so ok

drivers/block/drbd/drbd_main.c - uses bdev_physical_block_size(), so ok

drivers/block/loop.c - uses same size as LBS (which must be a 
power-of-2), so ok

drivers/block/mtip32xx/mtip32xx.c - uses 4096, so ok

drivers/block/n64cart.c - uses 4096, so ok

drivers/block/nbd.c - uses same size as LBS (which must be a 
power-of-2), so ok

drivers/block/null_blk/main.c - uses same size as LBS (which must be a
power-of-2), so ok

drivers/block/rnbd/rnbd-clt.c - drivers/block/rnbd/rnbd-srv.c - uses
bdev_physical_block_size() to fill in
rnbd_msg_open_rsp.physical_block_size, and rnbd-clt.c fills in
queue_limits.physical_block_size from
rnbd_msg_open_rsp.physical_block_size, so looks ok

drivers/block/sunvdc.c not sure on this one. For v1.2 spec we have 
vio_disk_attr_info.phys_block_size, but I cannot
find a spec detailing it.
https://oss.oracle.com/sparcdocs/hypervisor-api-3.0draft7.pdf has
earlier specs. FWIW, no rules on power-of-2 not mentioned for
vdisk_block_size in that spec, so unlikely to have rules for physical
block size. Default pbs is VDC_DEFAULT_BLK_SIZE = 512, and other sizes
in driver are all power-of-2, so likely phys_block_size
will be a power-of-2 always

drivers/block/ublk_drv.c - uses 1 << p->physical_bs_shift, so ok

drivers/block/virtio_blk.c - according to [0], should be ok

drivers/block/zloop.c - uses same size as LBS (which must be a
power-of-2), so ok

drivers/block/zram/zram_drv.c - uses PAGE_SIZE, so ok

drivers/md/bcache/super.c - uses same size as LBS (which must be a
power-of-2), so ok

drivers/md/dm-crypt.c - uses same size as LBS (which must be a
power-of-2), so ok

drivers/md/dm-ebs-target.c - ebs_ctr() -> __ebs_check_bs() ensures a
power-of-2, so ok

drivers/md/dm-integrity.c - uses same size as LBS (which must be a
power-of-2), so ok

drivers/md/dm-log-writes.c - uses bdev_physical_block_size(), so ok

drivers/md/dm-vdo/dm-vdo-target.c - VDO_BLOCK_SIZE = 4096, so ok

drivers/md/dm-verity-target.c - uses 1 << v->data_dev_block_bits, so ok

drivers/md/dm-writecache.c - writecache_ctr() line 2376 ensure
blocksize is a power-of-2, so ok

drivers/md/dm-zoned-target.c - uses same size as LBS (which must be a
power-of-2), so ok

drivers/nvdimm/pmem.c - uses PAGE_SIZE, so ok

drivers/nvme/host/core.c - may not be ok, so pbs comes from npwg and
spec does not mandate this is a power-of-2. According to [1], nothing
like this has been seen.

drivers/scsi/sd.c - uses (1 << (buffer[13] & 0xf)) * sector_size in
sdkp->physical_block_size, so ok

drivers/scsi/sd_zbc.c - uses sdkp->physical_block_size, so ok

drivers/target/target_core_iblock.c - uses bdev_physical_block_size(), so ok

[0] https://lore.kernel.org/linux-block/aa4b8526-5346-4004-a3b7-abcffd201b1a@kernel.org/
[1] https://lore.kernel.org/linux-block/yq1qzy8x5m8.fsf@ca-mkp.ca.oracle.com/

Changes since RFC:
- Add RB tags (thanks!)

John Garry (2):
  block: avoid possible overflow for chunk_sectors check in
    blk_stack_limits()
  block: Enforce power-of-2 physical block size

 block/blk-settings.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

-- 
2.43.5

[PATCH v2 1/2] block: avoid possible overflow for chunk_sectors check in blk_stack_limits()

[John Garry]

In blk_stack_limits(), we check that the t->chunk_sectors value is a
multiple of the t->physical_block_size value.

However, by finding the chunk_sectors value in bytes, we may overflow
the unsigned int which holds chunk_sectors, so change the check to be
based on sectors.

Reviewed-by: Hannes Reinecke <hare@suse.de>
Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-settings.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index a6ac293f47e34..fa53a330f9b99 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -795,7 +795,7 @@ int blk_stack_limits(struct queue_limits *t, struct queue_limits *b,
 	}
 
 	/* chunk_sectors a multiple of the physical block size? */
-	if ((t->chunk_sectors << 9) & (t->physical_block_size - 1)) {
+	if (t->chunk_sectors % (t->physical_block_size >> SECTOR_SHIFT)) {
 		t->chunk_sectors = 0;
 		t->flags |= BLK_FLAG_MISALIGNED;
 		ret = -1;
-- 
2.43.5

Re: [PATCH v2 1/2] block: avoid possible overflow for chunk_sectors check in blk_stack_limits()

[Damien Le Moal]

On 7/29/25 18:14, John Garry wrote:
> In blk_stack_limits(), we check that the t->chunk_sectors value is a
> multiple of the t->physical_block_size value.
> 
> However, by finding the chunk_sectors value in bytes, we may overflow
> the unsigned int which holds chunk_sectors, so change the check to be
> based on sectors.
> 
> Reviewed-by: Hannes Reinecke <hare@suse.de>
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: John Garry <john.g.garry@oracle.com>

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>


-- 
Damien Le Moal
Western Digital Research

[PATCH v2 2/2] block: Enforce power-of-2 physical block size

[John Garry]

The merging/splitting code and other queue limits checking depends on the
physical block size being a power-of-2, so enforce it.

Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
Signed-off-by: John Garry <john.g.garry@oracle.com>
---
 block/blk-settings.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index fa53a330f9b99..5ae0a253e43fd 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -274,6 +274,10 @@ int blk_validate_limits(struct queue_limits *lim)
 	}
 	if (lim->physical_block_size < lim->logical_block_size)
 		lim->physical_block_size = lim->logical_block_size;
+	else if (!is_power_of_2(lim->physical_block_size)) {
+		pr_warn("Invalid physical block size (%d)\n", lim->physical_block_size);
+		return -EINVAL;
+	}
 
 	/*
 	 * The minimum I/O size defaults to the physical block size unless
-- 
2.43.5

Re: [PATCH v2 2/2] block: Enforce power-of-2 physical block size

[Hannes Reinecke]

On 7/29/25 11:14, John Garry wrote:
> The merging/splitting code and other queue limits checking depends on the
> physical block size being a power-of-2, so enforce it.
> 
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>   block/blk-settings.c | 4 ++++
>   1 file changed, 4 insertions(+)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index fa53a330f9b99..5ae0a253e43fd 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -274,6 +274,10 @@ int blk_validate_limits(struct queue_limits *lim)
>   	}
>   	if (lim->physical_block_size < lim->logical_block_size)
>   		lim->physical_block_size = lim->logical_block_size;
> +	else if (!is_power_of_2(lim->physical_block_size)) {
> +		pr_warn("Invalid physical block size (%d)\n", lim->physical_block_size);
> +		return -EINVAL;
> +	}
>   
>   	/*
>   	 * The minimum I/O size defaults to the physical block size unless

Reviewed-by: Hannes Reinecke <hare@suse.de>

Cheers,

Hannes
-- 
Dr. Hannes Reinecke                  Kernel Storage Architect
hare@suse.de                                +49 911 74053 688
SUSE Software Solutions GmbH, Frankenstr. 146, 90461 Nürnberg
HRB 36809 (AG Nürnberg), GF: I. Totev, A. McDonald, W. Knoblich

Re: [PATCH v2 2/2] block: Enforce power-of-2 physical block size

[Damien Le Moal]

On 7/29/25 18:14, John Garry wrote:
> The merging/splitting code and other queue limits checking depends on the
> physical block size being a power-of-2, so enforce it.
> 
> Reviewed-by: Martin K. Petersen <martin.petersen@oracle.com>
> Signed-off-by: John Garry <john.g.garry@oracle.com>
> ---
>  block/blk-settings.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index fa53a330f9b99..5ae0a253e43fd 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -274,6 +274,10 @@ int blk_validate_limits(struct queue_limits *lim)
>  	}
>  	if (lim->physical_block_size < lim->logical_block_size)
>  		lim->physical_block_size = lim->logical_block_size;
> +	else if (!is_power_of_2(lim->physical_block_size)) {
> +		pr_warn("Invalid physical block size (%d)\n", lim->physical_block_size);

Nit: format should use %u

> +		return -EINVAL;
> +	}

Nit: Add the curly brackets to the if part too.

Other than that, looks OK to me.

Reviewed-by: Damien Le Moal <dlemoal@kernel.org>

>  
>  	/*
>  	 * The minimum I/O size defaults to the physical block size unless


-- 
Damien Le Moal
Western Digital Research

Re: [PATCH v2 0/2] block: make some queue limits checks more robust

[Jens Axboe]

On Tue, 29 Jul 2025 09:14:46 +0000, John Garry wrote:
> This series contains a couple of changes to make request queue limits
> checks more robust.
> 
> About the change to enforce a power-of-2 physical block size, I audited
> the drivers which set this. All look ok to comply with this rule, as
> follows:
> 
> [...]

Applied, thanks!

[1/2] block: avoid possible overflow for chunk_sectors check in blk_stack_limits()
      (no commit info)
[2/2] block: Enforce power-of-2 physical block size
      (no commit info)

Best regards,
-- 
Jens Axboe