Skip to content

Commit ff63198

Browse files
committed
Revert "virtio-blk: Add validation for block size in config space"
It turns out that access to config space before completing the feature negotiation is broken for big endian guests at least with QEMU hosts up to 6.1 inclusive. This affects any device that accesses config space in the validate callback: at the moment that is virtio-net with VIRTIO_NET_F_MTU but since 82e89ea ("virtio-blk: Add validation for block size in config space") that also started affecting virtio-blk with VIRTIO_BLK_F_BLK_SIZE. Further, unlike VIRTIO_NET_F_MTU which is off by default on QEMU, VIRTIO_BLK_F_BLK_SIZE is on by default, which resulted in lots of people not being able to boot VMs on BE. The spec is very clear that what we are doing is legal so QEMU needs to be fixed, but given it's been broken for so many years and no one noticed, we need to give QEMU a bit more time before applying this. Further, this patch is incomplete (does not check blk size is a power of two) and it duplicates the logic from nbd. Revert for now, and we'll reapply a cleaner logic in the next release. Cc: [email protected] Fixes: 82e89ea ("virtio-blk: Add validation for block size in config space") Cc: Xie Yongji <[email protected]> Signed-off-by: Michael S. Tsirkin <[email protected]>
1 parent 97f854b commit ff63198

File tree

1 file changed

+6
-31
lines changed

1 file changed

+6
-31
lines changed

drivers/block/virtio_blk.c

Lines changed: 6 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -689,28 +689,6 @@ static const struct blk_mq_ops virtio_mq_ops = {
689689
static unsigned int virtblk_queue_depth;
690690
module_param_named(queue_depth, virtblk_queue_depth, uint, 0444);
691691

692-
static int virtblk_validate(struct virtio_device *vdev)
693-
{
694-
u32 blk_size;
695-
696-
if (!vdev->config->get) {
697-
dev_err(&vdev->dev, "%s failure: config access disabled\n",
698-
__func__);
699-
return -EINVAL;
700-
}
701-
702-
if (!virtio_has_feature(vdev, VIRTIO_BLK_F_BLK_SIZE))
703-
return 0;
704-
705-
blk_size = virtio_cread32(vdev,
706-
offsetof(struct virtio_blk_config, blk_size));
707-
708-
if (blk_size < SECTOR_SIZE || blk_size > PAGE_SIZE)
709-
__virtio_clear_bit(vdev, VIRTIO_BLK_F_BLK_SIZE);
710-
711-
return 0;
712-
}
713-
714692
static int virtblk_probe(struct virtio_device *vdev)
715693
{
716694
struct virtio_blk *vblk;
@@ -722,6 +700,12 @@ static int virtblk_probe(struct virtio_device *vdev)
722700
u8 physical_block_exp, alignment_offset;
723701
unsigned int queue_depth;
724702

703+
if (!vdev->config->get) {
704+
dev_err(&vdev->dev, "%s failure: config access disabled\n",
705+
__func__);
706+
return -EINVAL;
707+
}
708+
725709
err = ida_simple_get(&vd_index_ida, 0, minor_to_index(1 << MINORBITS),
726710
GFP_KERNEL);
727711
if (err < 0)
@@ -836,14 +820,6 @@ static int virtblk_probe(struct virtio_device *vdev)
836820
else
837821
blk_size = queue_logical_block_size(q);
838822

839-
if (blk_size < SECTOR_SIZE || blk_size > PAGE_SIZE) {
840-
dev_err(&vdev->dev,
841-
"block size is changed unexpectedly, now is %u\n",
842-
blk_size);
843-
err = -EINVAL;
844-
goto out_cleanup_disk;
845-
}
846-
847823
/* Use topology information if available */
848824
err = virtio_cread_feature(vdev, VIRTIO_BLK_F_TOPOLOGY,
849825
struct virtio_blk_config, physical_block_exp,
@@ -1009,7 +985,6 @@ static struct virtio_driver virtio_blk = {
1009985
.driver.name = KBUILD_MODNAME,
1010986
.driver.owner = THIS_MODULE,
1011987
.id_table = id_table,
1012-
.validate = virtblk_validate,
1013988
.probe = virtblk_probe,
1014989
.remove = virtblk_remove,
1015990
.config_changed = virtblk_config_changed,

0 commit comments

Comments
 (0)