On Tue, Oct 10, 2023 at 3:58 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote: > > On Tue, 10 Oct 2023 14:41:39 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote: > > On Tue, Oct 10, 2023 at 11:11 AM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote: > > > > > > Some buggy devices, the common cfg size may not match the features. > > > > > > This patch checks the common cfg size for the > > > features(VIRTIO_F_NOTIF_CONFIG_DATA, VIRTIO_F_RING_RESET). When the > > > common cfg size does not match the corresponding feature, we fail the > > > probe and print error message. > > > > > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> > > > --- > > > drivers/virtio/virtio_pci_modern.c | 36 ++++++++++++++++++++++++++ > > > drivers/virtio/virtio_pci_modern_dev.c | 2 +- > > > include/linux/virtio_pci_modern.h | 1 + > > > 3 files changed, 38 insertions(+), 1 deletion(-) > > > > > > diff --git a/drivers/virtio/virtio_pci_modern.c b/drivers/virtio/virtio_pci_modern.c > > > index d6bb68ba84e5..6a8f5ff05636 100644 > > > --- a/drivers/virtio/virtio_pci_modern.c > > > +++ b/drivers/virtio/virtio_pci_modern.c > > > @@ -39,6 +39,39 @@ static void vp_transport_features(struct virtio_device *vdev, u64 features) > > > __virtio_set_bit(vdev, VIRTIO_F_RING_RESET); > > > } > > > > > > +static int __vp_check_common_size_one_feature(struct virtio_device *vdev, u32 fbit, > > > + u32 offset, const char *fname) > > > +{ > > > + struct virtio_pci_device *vp_dev = to_vp_device(vdev); > > > + > > > + if (!__virtio_test_bit(vdev, fbit)) > > > + return 0; > > > + > > > + if (likely(vp_dev->mdev.common_len >= offset)) > > > + return 0; > > > + > > > + dev_err(&vdev->dev, > > > + "virtio: common cfg size(%ld) does not match the feature %s\n", > > > + vp_dev->mdev.common_len, fname); > > > + > > > + return -EINVAL; > > > +} > > > + > > > +#define vp_check_common_size_one_feature(vdev, fbit, field) \ > > > + __vp_check_common_size_one_feature(vdev, fbit, \ > > > + offsetofend(struct virtio_pci_modern_common_cfg, field), #fbit) > > > + > > > +static int vp_check_common_size(struct virtio_device *vdev) > > > +{ > > > + if (vp_check_common_size_one_feature(vdev, VIRTIO_F_NOTIF_CONFIG_DATA, queue_notify_data)) > > > + return -EINVAL; > > > + > > > + if (vp_check_common_size_one_feature(vdev, VIRTIO_F_RING_RESET, queue_reset)) > > > + return -EINVAL; > > > > Do we need to at least check the offset of the queue_device as well here? > > > Not need. > > > /* > * vp_modern_map_capability - map a part of virtio pci capability > * @mdev: the modern virtio-pci device > * @off: offset of the capability > * @minlen: minimal length of the capability > * @align: align requirement > * @start: start from the capability > * @size: map size > * @len: the length that is actually mapped > * @pa: physical address of the capability > * > * Returns the io address of for the part of the capability > */ > static void __iomem * > vp_modern_map_capability(struct virtio_pci_modern_device *mdev, int off, > size_t minlen, u32 align, u32 start, u32 size, > size_t *len, resource_size_t *pa) > > > > caller: > mdev->common = vp_modern_map_capability(mdev, common, > sizeof(struct virtio_pci_common_cfg), 4, > 0, sizeof(struct virtio_pci_modern_common_cfg), > &mdev->common_len, NULL); > > > We pass the sizeof(struct virtio_pci_common_cfg) as the minlen. > > So we do not need to check the common cfg size is smaller then > sizeof(struct virtio_pci_common_cfg). Great. Acked-by: Jason Wang <jasowang@xxxxxxxxxx> Thanks > > Thanks. > > > > > Thanks > > > > > + > > > + return 0; > > > +} > > > + > > > /* virtio config->finalize_features() implementation */ > > > static int vp_finalize_features(struct virtio_device *vdev) > > > { > > > @@ -57,6 +90,9 @@ static int vp_finalize_features(struct virtio_device *vdev) > > > return -EINVAL; > > > } > > > > > > + if (vp_check_common_size(vdev)) > > > + return -EINVAL; > > > + > > > vp_modern_set_features(&vp_dev->mdev, vdev->features); > > > > > > return 0; > > > diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c > > > index 9cb601e16688..33f319da1558 100644 > > > --- a/drivers/virtio/virtio_pci_modern_dev.c > > > +++ b/drivers/virtio/virtio_pci_modern_dev.c > > > @@ -292,7 +292,7 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev) > > > mdev->common = vp_modern_map_capability(mdev, common, > > > sizeof(struct virtio_pci_common_cfg), 4, > > > 0, sizeof(struct virtio_pci_modern_common_cfg), > > > - NULL, NULL); > > > + &mdev->common_len, NULL); > > > if (!mdev->common) > > > goto err_map_common; > > > mdev->isr = vp_modern_map_capability(mdev, isr, sizeof(u8), 1, > > > diff --git a/include/linux/virtio_pci_modern.h b/include/linux/virtio_pci_modern.h > > > index 067ac1d789bc..edf62bae0474 100644 > > > --- a/include/linux/virtio_pci_modern.h > > > +++ b/include/linux/virtio_pci_modern.h > > > @@ -28,6 +28,7 @@ struct virtio_pci_modern_device { > > > /* So we can sanity-check accesses. */ > > > size_t notify_len; > > > size_t device_len; > > > + size_t common_len; > > > > > > /* Capability for when we need to map notifications per-vq. */ > > > int notify_map_cap; > > > -- > > > 2.32.0.3.g01195cf9f > > > > > > > _______________________________________________ > > Virtualization mailing list > > Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx > > https://lists.linuxfoundation.org/mailman/listinfo/virtualization > _______________________________________________ Virtualization mailing list Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linuxfoundation.org/mailman/listinfo/virtualization