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 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?


> 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.



-- 
MST