On Thu, Sep 29, 2022 at 10:29:09AM +0300, Alvaro Karsz wrote:
> > OK so virtio_blk_config->max_discard_seg is unused without
> > VIRTIO_BLK_F_DISCARD.
>
>
> Yes, if I understood the spec correctly,
> virtio_blk_config->max_discard_seg is relevant if VIRTIO_BLK_F_DISCARD
> is negotiated, and virtio_blk_config->max_secure_erase_seg is relevant
> if VIRTIO_BLK_F_SECURE_ERASE is negotiated.
>
> What should I do?
> Should I fix the patch?
I don't know. You guys are storage experts I'm a virtio guy.
And from virtio POV I have a question about this code:
+ virtio_cread(vdev, struct virtio_blk_config,
+ secure_erase_sector_alignment, &v);
+
+ /* secure_erase_sector_alignment should not be zero, the device should set a
+ * valid number of sectors.
+ */
+ if (!v) {
+ dev_err(&vdev->dev,
+ "virtio_blk: secure_erase_sector_alignment can't be 0\n");
+ err = -EINVAL;
+ goto out_cleanup_disk;
+ }
So this will prevent us from ever exposing a device
with secure_erase_sector_alignment set to 0.
Same for max_secure_erase_sectors and max_secure_erase_seg.
What can the value 0 mean here? I don't know, maybe "get actual
value from some other place".
An alternative is to put this code in a validate
callback and clear VIRTIO_BLK_F_SECURE_ERASE.
However, this means that even if host exposes VIRTIO_BLK_F_SECURE_ERASE
the host can not be sure guest will use secure erase.
Is this or can be a security problem?
If yes let's be strict and fail probe as current code does.
If not let's be flexible and ensure forward compatibility.
--
MST
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
