Re: Should there be a mode in which the virtqueue -> MSI mapping is fixed?

["Michael S. Tsirkin" <mst@redhat.com>]

On Sun, Apr 05, 2026 at 06:28:56PM -0400, Demi Marie Obenour wrote:
> On 4/5/26 17:50, Michael S. Tsirkin wrote:
> > On Sun, Apr 05, 2026 at 05:47:19PM -0400, Demi Marie Obenour wrote:
> >> On 4/5/26 17:09, Michael S. Tsirkin wrote:
> >>> On Sun, Apr 05, 2026 at 04:58:39PM -0400, Demi Marie Obenour wrote:
> >>>> On 4/5/26 16:15, Michael S. Tsirkin wrote:
> >>>>> On Sun, Apr 05, 2026 at 01:50:25PM -0400, Demi Marie Obenour wrote:
> >>>>>> On 4/4/26 20:56, Michael S. Tsirkin wrote:
> >>>>>>> On Sat, Apr 04, 2026 at 05:19:41PM -0400, Demi Marie Obenour wrote:
> >>>>>>>> Cloud Hypervisor's vhost-user frontend does not implement MSI-X
> >>>>>>>> properly [1].  Specifically:
> >>>>>>>>
> >>>>>>>> 1. Reads from the Pending Bit Array (PBA) always return 0.
> >>>>>>>> 2. Changes to the MSI associated with a virtqueue after the device
> >>>>>>>>    is activated are ignored.
> >>>>>>>>    
> >>>>>>>> Amazingly, there have not been any reports of this causing breakage.
> >>>>>>>> I have a fix for the first [2], which actually decreases the amount
> >>>>>>>> of code.  However, the second is trickier and I'm tempted to not
> >>>>>>>> bother unless it causes real-world problems.
> >>>>>>>>
> >>>>>>>> Are there real-world drivers that will run into either of the above
> >>>>>>>> bugs?  Linux seems to only choose anything else as a fallback, which
> >>>>>>>> presumably is not triggered.
> >>>>>>>>
> >>>>>>>> [1]: https://github.com/cloud-hypervisor/cloud-hypervisor/issues/7813
> >>>>>>>> [2]: https://github.com/cloud-hypervisor/cloud-hypervisor/pull/7963
> >>>>>>>
> >>>>>>> It will sometimes trigger.
> >>>>>>
> >>>>>> Would it be possible to provide an example?  A reproducible test
> >>>>>> case would be ideal, but conditions under which this will trigger
> >>>>>> are also sufficient.
> >>>>>
> >>>>>
> >>>>> I am not sure what does "is activated" mean.
> >>>>> For example, on latest Linux:
> >>>>>
> >>>>>         vq = vp_find_one_vq_msix(vdev, avq->vq_index, vp_modern_avq_done,
> >>>>>                                  avq->name, false, true, &allocated_vectors,
> >>>>>                                  vector_policy, &vp_dev->admin_vq.info);
> >>>>>         if (IS_ERR(vq)) {
> >>>>>                 err = PTR_ERR(vq);
> >>>>>                 goto error_find;
> >>>>>         }
> >>>>>
> >>>>>         return 0;
> >>>>>
> >>>>> error_find:
> >>>>>         vp_del_vqs(vdev);
> >>>>>         return err;
> >>>>> }
> >>>>>
> >>>>>
> >>>>> And 
> >>>>>
> >>>>> static void del_vq(struct virtio_pci_vq_info *info)
> >>>>> {
> >>>>>         struct virtqueue *vq = info->vq;
> >>>>>         struct virtio_pci_device *vp_dev = to_vp_device(vq->vdev);
> >>>>>         struct virtio_pci_modern_device *mdev = &vp_dev->mdev;
> >>>>>
> >>>>>         if (vp_dev->msix_enabled)
> >>>>>                 vp_modern_queue_vector(mdev, vq->index,
> >>>>>                                        VIRTIO_MSI_NO_VECTOR);
> >>>>>
> >>>>>         if (!mdev->notify_base)
> >>>>>                 pci_iounmap(mdev->pci_dev, (void __force __iomem *)vq->priv);
> >>>>>
> >>>>>         vring_del_virtqueue(vq);
> >>>>> }
> >>>>>
> >>>>>
> >>>>> and this happens after feature negotiation.
> >>>>>
> >>>>> It's before device_ready, however.
> >>>>
> >>>> Are there drivers that will change virtqueue => MSI-X vector mappings
> >>>> after DRIVER_OK without an intervening reset?  Cloud Hypervisor
> >>>> supports this for devices it implements internally, but it ignores
> >>>> such changes for vhost-user devices.  Is this going to cause problems
> >>>> in practice?
> >>>
> >>>
> >>> Also yes.
> >>>
> >>> For example, dpdk uses this during cleanup to block interrupts:
> >>> drivers/net/virtio/virtio_ethdev.c
> >>>
> >>>
> >>> int
> >>> virtio_dev_close(struct rte_eth_dev *dev)
> >>> {       
> >>>         struct virtio_hw *hw = dev->data->dev_private;
> >>>         struct rte_eth_intr_conf *intr_conf = &dev->data->dev_conf.intr_conf;
> >>>                                              
> >>>         PMD_INIT_LOG(DEBUG, "virtio_dev_close");
> >>>         if (rte_eal_process_type() != RTE_PROC_PRIMARY)
> >>>                 return 0;
> >>>
> >>>         if (!hw->opened)
> >>>                 return 0;
> >>>         hw->opened = 0;
> >>>
> >>>         /* reset the NIC */
> >>>         if (dev->data->dev_flags & RTE_ETH_DEV_INTR_LSC)
> >>>                 VIRTIO_OPS(hw)->set_config_irq(hw, VIRTIO_MSI_NO_VECTOR);
> >>>         if (intr_conf->rxq)
> >>>                 virtio_queues_unbind_intr(dev);
> >>>
> >>>         if (intr_conf->lsc || intr_conf->rxq) {
> >>>                 virtio_intr_disable(dev);
> >>>                 rte_intr_efd_disable(dev->intr_handle);
> >>>                 rte_intr_vec_list_free(dev->intr_handle);
> >>>         }
> >>>
> >>>         virtio_reset(hw);
> >>>         virtio_dev_free_mbufs(dev);
> >>>         virtio_free_queues(hw);
> >>>         virtio_free_rss(hw);
> >>>
> >>>         return VIRTIO_OPS(hw)->dev_close(hw);
> >>> }
> >>
> >> What would happen if DPDK received a spurious MSI-X interrupt during
> >> this time?
> > 
> > Could be a UAF?
> 
> Indeed it could, provided that DPDK is actually able to get the interrupt.
> 
> >> I know that this is non-compliant with the virtio specification.
> >> However, a problem that will trigger in practice, or which can be
> >> used for an exploit, is far more severe (and thus far more important
> >> to fix) than one that does not have practical consequences.
> >> -- 
> >> Sincerely,
> >> Demi Marie Obenour (she/her/hers)
> > 
> > 
> > 
> > You will have to read the code, and lots of old versions of it, to
> > check.
> 
> Makes sense.
> 
> Right now, I'm working a spec for a new device (vhost-guest, formally
> virtio-vhost-user).  It's a vhost-user server, so it is a virtio
> device used by one guest VM (backend) to implement a virtio device
> for use by another guest VM (frontend).  Yes, it's confusing!
> 
> The spec I am using as a basis is
> <https://stefanha.github.io/virtio/vhost-user-slave.html>.  Both its
> terminology and the spec itself are outdated and I'm going to be
> heavily changing it before submission.  The current (unpublished)
> version states that a vhost-guest device has 4 virtqueues in two pairs.
> One pair is for requests from frontend to backend and responses
> from backend to frontend.  The other is for requests from backend
> to frontend and responses from frontend to backend.  However, the
> device that the driver is implementing *also* has its own virtqueues,
> and those need extra notifications.
> 
> The spec I am using as reference states that these notifications are
> treated similar to existing virtqueue notifications.  In particular,
> the MSI-X vector associated with them can be changed at will.
> How important is it to support this in the version of the spec
> I submit?  How important is it to allow the same MSI-X vector to be
> used for multiple notifications?
> -- 
> Sincerely,
> Demi Marie Obenour (she/her/hers)

We can have a modified pci transport that doesn't allow this - just
reserve another range of IDs for that. There are a couple of other
spec mistakes we can correct if we do this.


I think the more important issue is disabling interrupts through setting
vector to 0xffff. That does not have a direct equivalent in the pci
spec.



-- 
MST