| Message ID | 20240524104651.92506-1-hare@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | [PATCHv2] block: check for max_hw_sectors underflow | expand |
Looks good:
Reviewed-by: Christoph Hellwig <hch@lst.de>
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
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.
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()
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
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(-)