search for: blk_is_valid_logical_block_size

...ogical_block_size - check if logical block size is supported > > + * by the kernel > > + * @size: the logical block size, in bytes > > + * > > + * Description: > > + * This function checks if the block layers supports given block size > > + **/ > > +bool blk_is_valid_logical_block_size(unsigned int size) > > +{ > > + return size >= SECTOR_SIZE && size <= PAGE_SIZE && !is_power_of_2(size); Note here a typo, made in last minute change which I didn't test. It should be without '!' Best regards, Maxim Levitsky > > +} > > +E...

...} EXPORT_SYMBOL(blk_queue_max_segment_size); + +/** + * blk_check_logical_block_size - check if logical block size is supported + * by the kernel + * @size: the logical block size, in bytes + * + * Description: + * This function checks if the block layers supports given block size + **/ +bool blk_is_valid_logical_block_size(unsigned int size) +{ + return size >= SECTOR_SIZE && size <= PAGE_SIZE && !is_power_of_2(size); +} +EXPORT_SYMBOL(blk_is_valid_logical_block_size); + /** * blk_queue_logical_block_size - set logical block size for the queue * @q: the request queue for the device @@ -323...

> +/** > + * blk_check_logical_block_size - check if logical block size is supported > + * by the kernel > + * @size: the logical block size, in bytes > + * > + * Description: > + * This function checks if the block layers supports given block size > + **/ > +bool blk_is_valid_logical_block_size(unsigned int size) > +{ > + return size >= SECTOR_SIZE && size <= PAGE_SIZE && !is_power_of_2(size); Shouldn't this be a ... && is_power_of_2(size)? > if (q->limits.io_min < q->limits.physical_block_size) > q->limits.io_min = q->li...

...eck for the block size relatively early, often store it in some internal struct etc, prior to calling blk_queue_logical_block_size thus making them only to rely on blk_queue_logical_block_size as the check for block size validity will need non-trivial changes in their code. Instead of this adding blk_is_valid_logical_block_size allowed me to trivially convert most of the uses. For RFC I converted only some drivers that I am more familiar with and/or can test but I can remove the driver's own checks in most other drivers with low chance of introducing a bug, even if I can't test the driver. What do you think? I...

...b *nullb, struct blk_mq_tag_set *set) > > > > static int null_validate_conf(struct nullb_device *dev) > > { > > - dev->blocksize = round_down(dev->blocksize, 512); > > - dev->blocksize = clamp_t(unsigned int, dev->blocksize, 512, 4096); > > + if (!blk_is_valid_logical_block_size(dev->blocksize)) > > + return -ENODEV; > > > > if (dev->queue_mode == NULL_Q_MQ && dev->use_per_node_hctx) { > > if (dev->submit_queues != nr_online_nodes) > > @@ -1865,7 +1865,7 @@ static int __init null_init(void) > > struct nullb...

On Tue, 2020-07-21 at 11:25 +0000, Damien Le Moal wrote: > On 2020/07/21 19:55, Maxim Levitsky wrote: > > Use blk_is_valid_logical_block_size instead of hardcoded list > > s/hardcoded list/hardcoded checks./ Done, thanks! Best regards, Maxim Levitsky > > > Signed-off-by: Maxim Levitsky <mlevitsk at redhat.com> > > --- > > drivers/scsi/sd.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deleti...

...b/drivers/block/virtio_blk.c > @@ -809,10 +809,18 @@ static int virtblk_probe(struct virtio_device *vdev) > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE, > struct virtio_blk_config, blk_size, > &blk_size); > - if (!err) > + if (!err) { > + if (!blk_is_valid_logical_block_size(blk_size)) { > + dev_err(&vdev->dev, > + "%s failure: invalid logical block size %d\n", > + __func__, blk_size); > + err = -EINVAL; > + goto out_cleanup_queue; > + } > blk_queue_logical_block_size(q, blk_size); Hmm, I wonder if we should simply...

...b/drivers/block/virtio_blk.c > @@ -809,10 +809,18 @@ static int virtblk_probe(struct virtio_device *vdev) > err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE, > struct virtio_blk_config, blk_size, > &blk_size); > - if (!err) > + if (!err) { > + if (!blk_is_valid_logical_block_size(blk_size)) { > + dev_err(&vdev->dev, > + "%s failure: invalid logical block size %d\n", > + __func__, blk_size); > + err = -EINVAL; > + goto out_cleanup_queue; > + } > blk_queue_logical_block_size(q, blk_size); Hmm, I wonder if we should simply...

This patch series aims to move the logical block size checking to the block code. This was inspired by missing check for valid logical block size in virtio-blk which causes the kernel to crash in a weird way later on when it is invalid. I added blk_is_valid_logical_block_size which returns true iff the block size is one of supported sizes. I added this check to virtio-blk, and also converted few block drivers that I am familiar with to use this interface instead of their own implementation. Best regards, Maxim Levitsky Maxim Levitsky (10): block: introduce blk_is...

This patch series aims to move the logical block size checking to the block code. This was inspired by missing check for valid logical block size in virtio-blk which causes the kernel to crash in a weird way later on when it is invalid. I added blk_is_valid_logical_block_size which returns true iff the block size is one of supported sizes. I added this check to virtio-blk, and also converted few block drivers that I am familiar with to use this interface instead of their own implementation. Best regards, Maxim Levitsky Maxim Levitsky (10): block: introduce blk_is...

...-- a/drivers/block/virtio_blk.c +++ b/drivers/block/virtio_blk.c @@ -809,10 +809,18 @@ static int virtblk_probe(struct virtio_device *vdev) err = virtio_cread_feature(vdev, VIRTIO_BLK_F_BLK_SIZE, struct virtio_blk_config, blk_size, &blk_size); - if (!err) + if (!err) { + if (!blk_is_valid_logical_block_size(blk_size)) { + dev_err(&vdev->dev, + "%s failure: invalid logical block size %d\n", + __func__, blk_size); + err = -EINVAL; + goto out_cleanup_queue; + } blk_queue_logical_block_size(q, blk_size); - else + } else { blk_size = queue_logical_block_size(q); + }...