lkml.org 
[lkml]   [2020]   [Jul]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
SubjectRe: [PATCH] virtio-blk: check host supplied logical block size
From
Date
On Wed, 2020-07-15 at 06:06 -0400, Michael S. Tsirkin wrote:
> On Wed, Jul 15, 2020 at 12:55:18PM +0300, Maxim Levitsky wrote:
> > Linux kernel only supports logical block sizes which are power of
> > two,
> > at least 512 bytes and no more that PAGE_SIZE.
> >
> > Check this instead of crashing later on.
> >
> > Note that there is no need to check physical block size since it is
> > only a hint, and virtio-blk already only supports power of two
> > values.
> >
> > Bugzilla link: https://bugzilla.redhat.com/show_bug.cgi?id=1664619
> >
> > Signed-off-by: Maxim Levitsky <mlevitsk@redhat.com>
> > ---
> > drivers/block/virtio_blk.c | 20 ++++++++++++++++++--
> > 1 file changed, 18 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c
> > index 980df853ee497..36dda31cc4e96 100644
> > --- a/drivers/block/virtio_blk.c
> > +++ b/drivers/block/virtio_blk.c
> > @@ -681,6 +681,12 @@ static const struct blk_mq_ops virtio_mq_ops =
> > {
> > static unsigned int virtblk_queue_depth;
> > module_param_named(queue_depth, virtblk_queue_depth, uint, 0444);
> >
> > +
> > +static bool virtblk_valid_block_size(unsigned int blksize)
> > +{
> > + return blksize >= 512 && blksize <= PAGE_SIZE &&
> > is_power_of_2(blksize);
> > +}
> > +
>
> Is this a blk core assumption? in that case, does not this belong
> in blk core?

It is a blk core assumption.
I had checked other drivers and these that have variable block size all
check this manually like that.

I don't mind fixing all of them but I am a bit afraid to create
too much mess.

>
> > static int virtblk_probe(struct virtio_device *vdev)
> > {
> > struct virtio_blk *vblk;
> > @@ -809,9 +815,16 @@ 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 (!virtblk_valid_block_size(blk_size)) {
> > + dev_err(&vdev->dev,
> > + "%s failure: unsupported 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);
> >
> > /* Use topology information if available */
>
> OK so if we are doing this pls add {} around blk_size =
> queue_logical_block_size(q);
> too.
Will do.

>
> > @@ -872,6 +885,9 @@ static int virtblk_probe(struct virtio_device
> > *vdev)
> > device_add_disk(&vdev->dev, vblk->disk, virtblk_attr_groups);
> > return 0;
> >
> > +out_cleanup_queue:
> > + blk_cleanup_queue(vblk->disk->queue);
> > + vblk->disk->queue = NULL;
> > out_free_tags:
> > blk_mq_free_tag_set(&vblk->tag_set);
> > out_put_disk:
> > --
> > 2.26.2


Best regards,
Maxim Levitsky

\
 
 \ /
  Last update: 2020-07-15 12:20    [W:0.202 / U:0.272 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site