On Mon, Oct 4, 2021 at 11:39 PM Michael S. Tsirkin <mst@xxxxxxxxxx> wrote: > > On Mon, Oct 04, 2021 at 11:27:29AM -0400, Michael S. Tsirkin wrote: > > On Mon, Aug 09, 2021 at 06:16:09PM +0800, Xie Yongji wrote: > > > An untrusted device might presents an invalid block size > > > in configuration space. This tries to add validation for it > > > in the validate callback and clear the VIRTIO_BLK_F_BLK_SIZE > > > feature bit if the value is out of the supported range. > > > > > > And we also double check the value in virtblk_probe() in > > > case that it's changed after the validation. > > > > > > Signed-off-by: Xie Yongji <xieyongji@xxxxxxxxxxxxx> > > > > So I had to revert this due basically bugs in QEMU. > > > > My suggestion at this point is to try and update > > blk_queue_logical_block_size to BUG_ON when the size > > is out of a reasonable range. > > > > This has the advantage of fixing more hardware, not just virtio. > > > > > > > > > --- > > > drivers/block/virtio_blk.c | 39 +++++++++++++++++++++++++++++++++------ > > > 1 file changed, 33 insertions(+), 6 deletions(-) > > > > > > diff --git a/drivers/block/virtio_blk.c b/drivers/block/virtio_blk.c > > > index 4b49df2dfd23..afb37aac09e8 100644 > > > --- a/drivers/block/virtio_blk.c > > > +++ b/drivers/block/virtio_blk.c > > > @@ -692,6 +692,28 @@ 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 int virtblk_validate(struct virtio_device *vdev) > > > +{ > > > + u32 blk_size; > > > + > > > + if (!vdev->config->get) { > > > + dev_err(&vdev->dev, "%s failure: config access disabled\n", > > > + __func__); > > > + return -EINVAL; > > > + } > > > + > > > + if (!virtio_has_feature(vdev, VIRTIO_BLK_F_BLK_SIZE)) > > > + return 0; > > > + > > > + blk_size = virtio_cread32(vdev, > > > + offsetof(struct virtio_blk_config, blk_size)); > > > + > > > + if (blk_size < SECTOR_SIZE || blk_size > PAGE_SIZE) > > > + __virtio_clear_bit(vdev, VIRTIO_BLK_F_BLK_SIZE); > > > + > > > + return 0; > > > +} > > > + > > > static int virtblk_probe(struct virtio_device *vdev) > > > { > > > struct virtio_blk *vblk; > > I started wondering about this. So let's assume is > PAGE_SIZE < blk_size (after all it's up to guest at many platforms). > > Will using the device even work given blk size is less than what > is can support? > > And what exactly happens today if blk_size is out of this range? > Now the block layer can't support the block size larger than the page size. Otherwise, it would cause a random crash, e.g. in block_read_full_page(). Thanks, Yongji