On Thu, Mar 05, 2026 at 04:48:29AM -0500, Michael S. Tsirkin wrote:
> On Thu, Mar 05, 2026 at 05:29:27PM +0800, Chaohai Chen wrote:
> > KCSAN detected multiple data races when accessing the split virtqueue's
> > used ring, which is shared memory concurrently accessed by both the CPU
> > and the virtio device (hypervisor).
> >
> > The races occur when reading the following fields without proper atomic
> > operations:
> > - vring.used->idx
> > - vring.used->flags
> > - vring.used->ring[].id
> > - vring.used->ring[].len
> >
> > These fields reside in DMA-shared memory and can be modified by the
> > virtio device at any time. Without READ_ONCE(), the compiler may perform
> > unsafe optimizations such as value caching or load tearing.
>
> .... but does not.
>
>
Sorry, you are right. There is virtio_rmb() doing synchronize.
> > @@ -1112,8 +1112,9 @@ static bool virtqueue_enable_cb_delayed_split(struct
> > vring_virtqueue *vq)
> > &vring_used_event(&vq->split.vring),
> > cpu_to_virtio16(vq->vq.vdev, vq->last_used_idx + bufs));
> >
> > - if (unlikely((u16)(virtio16_to_cpu(vq->vq.vdev,
> > vq->split.vring.used->idx)
> > - - vq->last_used_idx) > bufs)) {
> > + if (unlikely((u16)(virtio16_to_cpu(vq->vq.vdev,
> > + READ_ONCE(vq->split.vring.used->idx))
KSCAN just warned this code, and I understand virtio_store_mb() above it
can not synchronize here. Do you think we can just change this line ?
> > + - vq->last_used_idx) > bufs)) {
> > END_USE(vq);
> > return false;
