On Thu, 18 Aug 2022 16:40:23 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> On Thu, Aug 18, 2022 at 4:13 PM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
> >
> > On Thu, 18 Aug 2022 16:10:45 +0800, Jason Wang <jasowang@xxxxxxxxxx> wrote:
> > > On Thu, Aug 18, 2022 at 11:04 AM Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx> wrote:
> > > >
> > > > Legacy virtio pci has no way to communicate a change in vq size to
> > > > the hypervisor. If ring sizes don't match hypervisor will happily
> > > > corrupt memory.
> > > >
> > > > We add a check to vring size before calling
> > > > vp_legacy_set_queue_address(). Checking the memory range directly is a
> > > > bit cumbersome.
> > > >
> > > > Signed-off-by: Xuan Zhuo <xuanzhuo@xxxxxxxxxxxxxxxxx>
> > > > ---
> > > >
> > > > v2: replace BUG_ON with WARN_ON_ONCE. @Linus
> > > >
> > > > drivers/virtio/virtio_pci_legacy.c | 9 +++++++++
> > > > 1 file changed, 9 insertions(+)
> > > >
> > > > diff --git a/drivers/virtio/virtio_pci_legacy.c b/drivers/virtio/virtio_pci_legacy.c
> > > > index 2257f1b3d8ae..091e73d74e94 100644
> > > > --- a/drivers/virtio/virtio_pci_legacy.c
> > > > +++ b/drivers/virtio/virtio_pci_legacy.c
> > > > @@ -146,6 +146,15 @@ static struct virtqueue *setup_vq(struct virtio_pci_device *vp_dev,
> > > > goto out_del_vq;
> > > > }
> > > >
> > > > + /* Legacy virtio pci has no way to communicate a change in vq size to
> > > > + * the hypervisor. If ring sizes don't match hypervisor will happily
> > > > + * corrupt memory.
> > > > + */
> > > > + if (WARN_ON_ONCE(num != virtqueue_get_vring_size(vq))) {
> > > > + err = -EPERM;
> > > > + goto out_del_vq;
> > > > + }
> > >
> > > I wonder if it's better to have a num_min instead.
> >
> >
> > num_min?
> >
> > What is that?
>
> minimux size of a virtqueue since we had:
>
> if (num > vq->vq.num_max)
> return -E2BIG;
>
> in virtqueue_resize()
>
> we can then simply add
>
> if (num < vq->vq.num_min)
> return -EINVAL;
>
> etc?
This is not for resize.
Instead, it is used to check that the queue size of the ring created by virtio
is equal to the queue size provided by legacy pci. The two must be the same in
the case of legacy.
Thanks.
>
> Thanks
>
> >
> > Thanks.
> >
> > >
> > > Thanks
> > >
> > > > +
> > > > /* activate the queue */
> > > > vp_legacy_set_queue_address(&vp_dev->ldev, index, q_pfn);
> > > >
> > > > --
> > > > 2.31.0
> > > >
> > >
> >
>
_______________________________________________
Virtualization mailing list
Virtualization@xxxxxxxxxxxxxxxxxxxxxxxxxx
https://lists.linuxfoundation.org/mailman/listinfo/virtualization
