[PATCHv2] block: check for max_hw_sectors underflow

[PATCHv2] block: check for max_hw_sectors underflow

[Hannes Reinecke]

The logical block size need to be smaller than the max_hw_sector
setting, otherwise we can't even transfer a single LBA.

Signed-off-by: Hannes Reinecke <hare@kernel.org>
---
 block/blk-settings.c | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)

diff --git a/block/blk-settings.c b/block/blk-settings.c
index d2731843f2fc..030afb597183 100644
--- a/block/blk-settings.c
+++ b/block/blk-settings.c
@@ -104,6 +104,7 @@ static int blk_validate_zoned_limits(struct queue_limits *lim)
 static int blk_validate_limits(struct queue_limits *lim)
 {
 	unsigned int max_hw_sectors;
+	unsigned int logical_block_sectors;
 
 	/*
 	 * Unless otherwise specified, default to 512 byte logical blocks and a
@@ -134,8 +135,11 @@ static int blk_validate_limits(struct queue_limits *lim)
 		lim->max_hw_sectors = BLK_SAFE_MAX_SECTORS;
 	if (WARN_ON_ONCE(lim->max_hw_sectors < PAGE_SECTORS))
 		return -EINVAL;
+	logical_block_sectors = lim->logical_block_size >> SECTOR_SHIFT;
+	if (WARN_ON_ONCE(logical_block_sectors > lim->max_hw_sectors))
+		return -EINVAL;
 	lim->max_hw_sectors = round_down(lim->max_hw_sectors,
-			lim->logical_block_size >> SECTOR_SHIFT);
+			logical_block_sectors);
 
 	/*
 	 * The actual max_sectors value is a complex beast and also takes the
@@ -153,7 +157,7 @@ static int blk_validate_limits(struct queue_limits *lim)
 		lim->max_sectors = min(max_hw_sectors, BLK_DEF_MAX_SECTORS_CAP);
 	}
 	lim->max_sectors = round_down(lim->max_sectors,
-			lim->logical_block_size >> SECTOR_SHIFT);
+			logical_block_sectors);
 
 	/*
 	 * Random default for the maximum number of segments.  Driver should not
-- 
2.35.3

Re: [PATCHv2] block: check for max_hw_sectors underflow

[Christoph Hellwig]

Looks good:

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

Re: [PATCHv2] block: check for max_hw_sectors underflow

[John Garry]

On 24/05/2024 11:46, Hannes Reinecke wrote:
> The logical block size need to be smaller than the max_hw_sector
> setting, otherwise we can't even transfer a single LBA.
> 
> Signed-off-by: Hannes Reinecke <hare@kernel.org>

Regardless of comment, below:

Reviewed-by: John Garry <john.g.garry@oracle.com>

> ---
>   block/blk-settings.c | 8 ++++++--
>   1 file changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/block/blk-settings.c b/block/blk-settings.c
> index d2731843f2fc..030afb597183 100644
> --- a/block/blk-settings.c
> +++ b/block/blk-settings.c
> @@ -104,6 +104,7 @@ static int blk_validate_zoned_limits(struct queue_limits *lim)
>   static int blk_validate_limits(struct queue_limits *lim)
>   {
>   	unsigned int max_hw_sectors;
> +	unsigned int logical_block_sectors;
>   
>   	/*
>   	 * Unless otherwise specified, default to 512 byte logical blocks and a
> @@ -134,8 +135,11 @@ static int blk_validate_limits(struct queue_limits *lim)
>   		lim->max_hw_sectors = BLK_SAFE_MAX_SECTORS;
>   	if (WARN_ON_ONCE(lim->max_hw_sectors < PAGE_SECTORS))
>   		return -EINVAL;
> +	logical_block_sectors = lim->logical_block_size >> SECTOR_SHIFT;
> +	if (WARN_ON_ONCE(logical_block_sectors > lim->max_hw_sectors))
> +		return -EINVAL;
>   	lim->max_hw_sectors = round_down(lim->max_hw_sectors,

I don't think that we ever check if lim->logical_block_size is a 
power-of-2 - but that's a given, right?

> -			lim->logical_block_size >> SECTOR_SHIFT);
> +			logical_block_sectors);
>   
>   	/*
>   	 * The actual max_sectors value is a complex beast and also takes the
> @@ -153,7 +157,7 @@ static int blk_validate_limits(struct queue_limits *lim)
>   		lim->max_sectors = min(max_hw_sectors, BLK_DEF_MAX_SECTORS_CAP);
>   	}
>   	lim->max_sectors = round_down(lim->max_sectors,
> -			lim->logical_block_size >> SECTOR_SHIFT);
> +			logical_block_sectors);
>   
>   	/*
>   	 * Random default for the maximum number of segments.  Driver should not

Re: [PATCHv2] block: check for max_hw_sectors underflow

[Christoph Hellwig]

On Tue, May 28, 2024 at 11:54:42AM +0100, John Garry wrote:
> I don't think that we ever check if lim->logical_block_size is a power-of-2 
> - but that's a given, right?

It has to be for the block stack to work.  That being said now that we
do have a single good place for sanity checks it's probably worth to
add this check explicitly.

Re: [PATCHv2] block: check for max_hw_sectors underflow

[John Garry]

On 28/05/2024 11:58, Christoph Hellwig wrote:
> On Tue, May 28, 2024 at 11:54:42AM +0100, John Garry wrote:
>> I don't think that we ever check if lim->logical_block_size is a power-of-2
>> - but that's a given, right?
> It has to be for the block stack to work.  That being said now that we
> do have a single good place for sanity checks it's probably worth to
> add this check explicitly.

I think that we might be able to get rid of some of the driver 
blk_validate_block_size() calls (if we do that), like __nbd_set_size() 
-> blk_validate_block_size()