Re: [PATCH 1/2] virtio_pci: fix the common map size and add check for vq-reset

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Sep 25, 2023 at 11:58 AM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
>
> Now, the function vp_modern_map_capability() takes the size parameter,
> which corresponds to the size of virtio_pci_common_cfg. As a result, the
> pci_iomap_range() function maps the memory area for
> virtio_pci_common_cfg.
>
> However, if the _F_RING_RESET is negotiated, the extra items will be
> used. Therefore, we need to use the size of virtio_pci_modre_common_cfg
> to map more space.
>
> Meanwhile, I have introduced a new variable called common_len in the
> mdev. This allows us to check common_len when accessing the new item of
> virtio_pci_modre_common_cfg.
>
> Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> ---
>  drivers/virtio/virtio_pci_modern_dev.c | 8 ++++++--
>  include/linux/virtio_pci_modern.h      | 1 +
>  2 files changed, 7 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/virtio/virtio_pci_modern_dev.c b/drivers/virtio/virtio_pci_modern_dev.c
> index aad7d9296e77..ef6667de1b38 100644
> --- a/drivers/virtio/virtio_pci_modern_dev.c
> +++ b/drivers/virtio/virtio_pci_modern_dev.c
> @@ -291,8 +291,8 @@ int vp_modern_probe(struct virtio_pci_modern_device *mdev)
>         err = -EINVAL;
>         mdev->common = vp_modern_map_capability(mdev, common,
>                                       sizeof(struct virtio_pci_common_cfg), 4,
> -                                     0, sizeof(struct virtio_pci_common_cfg),
> -                                     NULL, NULL);
> +                                     0, sizeof(struct virtio_pci_modern_common_cfg),
> +                                     &mdev->common_len, NULL);
>         if (!mdev->common)
>                 goto err_map_common;
>         mdev->isr = vp_modern_map_capability(mdev, isr, sizeof(u8), 1,
> @@ -493,6 +493,8 @@ int vp_modern_get_queue_reset(struct virtio_pci_modern_device *mdev, u16 index)
>  {
>         struct virtio_pci_modern_common_cfg __iomem *cfg;
>
> +       BUG_ON(mdev->common_len < offsetofend(struct virtio_pci_modern_common_cfg, queue_reset));

Instead of using BUG for buggy hardware, why not simply fail the probe
in this case?

Thanks

> +
>         cfg = (struct virtio_pci_modern_common_cfg __iomem *)mdev->common;
>
>         vp_iowrite16(index, &cfg->cfg.queue_select);
> @@ -509,6 +511,8 @@ void vp_modern_set_queue_reset(struct virtio_pci_modern_device *mdev, u16 index)
>  {
>         struct virtio_pci_modern_common_cfg __iomem *cfg;
>
> +       BUG_ON(mdev->common_len < offsetofend(struct virtio_pci_modern_common_cfg, queue_reset));
> +
>         cfg = (struct virtio_pci_modern_common_cfg __iomem *)mdev->common;
>
>         vp_iowrite16(index, &cfg->cfg.queue_select);
> 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




[Index of Archives]     [KVM Development]     [Libvirt Development]     [Libvirt Users]     [CentOS Virtualization]     [Netdev]     [Ethernet Bridging]     [Linux Wireless]     [Kernel Newbies]     [Security]     [Linux for Hams]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux RAID]     [Linux Admin]     [Samba]

  Powered by Linux