linux-kernel - Re: [PATCH v2 1/2] virtio: silence KCSAN warning in virtqueue_get_buf_ctx

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20260128054415-mutt-send-email-mst@kernel.org>
Date: Wed, 28 Jan 2026 05:48:57 -0500
From: "Michael S. Tsirkin" <mst@...hat.com>
To: Alexander Graf <graf@...zon.com>
Cc: Johannes Thumshirn <Johannes.Thumshirn@....com>,
	"virtualization@...ts.linux.dev" <virtualization@...ts.linux.dev>,
	Jason Wang <jasowang@...hat.com>,
	Xuan Zhuo <xuanzhuo@...ux.alibaba.com>,
	Eugenio Pérez <eperezma@...hat.com>,
	"linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2 1/2] virtio: silence KCSAN warning in
 virtqueue_get_buf_ctx_split

On Wed, Jan 28, 2026 at 11:38:48AM +0100, Alexander Graf wrote:
> 
> On 28.01.26 11:34, Michael S. Tsirkin wrote:
> > On Wed, Jan 28, 2026 at 11:30:20AM +0100, Alexander Graf wrote:
> > > On 28.01.26 10:13, Johannes Thumshirn wrote:
> > > > On 1/28/26 10:03 AM, Alexander Graf wrote:
> > > > > On 28.01.26 09:47, Johannes Thumshirn wrote:
> > > > > > On 1/27/26 5:30 PM, Alexander Graf wrote:
> > > > > > > This patches the split vring format, but does not touch the packed one.
> > > > > > > What happens if you run the same test with the packed format? You can do
> > > > > > > so by passing "packed=on" as argument to your -device parameter.
> > > > > > This opened up a whole new can of worms... :(
> > > > > That's what I expected :).
> > > > > 
> > > > > How do other DMA based devices handle this? Is the real problem that
> > > > > virtio by default does not use the DMA API and so it confuses generic
> > > > > KCSAN logic that would otherwise track DMA regions as "can be modified
> > > > > by DMA at any time"?
> > > > > 
> > > > > If that is the case, maybe what we really want is to force enable use of
> > > > > the DMA API when KCSAN is active. Does something like the (whitespace
> > > > > broken) patch below work?
> > > > > 
> > > > > Alex
> > > > > 
> > > > > diff --git a/drivers/virtio/virtio_ring.c b/drivers/virtio/virtio_ring.c
> > > > > index ddab68959671..b1dd790ce622 100644
> > > > > --- a/drivers/virtio/virtio_ring.c
> > > > > +++ b/drivers/virtio/virtio_ring.c
> > > > > @@ -284,6 +284,13 @@ static bool vring_use_map_api(const struct
> > > > > virtio_device *vdev)
> > > > >          if (xen_domain())
> > > > >              return true;
> > > > > 
> > > > > +    /*
> > > > > +     * KCSAN needs to track who can modify memory. DMA API gets
> > > > > +     * us that, so always use it.
> > > > > +     */
> > > > > +    if (IS_ENABLED(CONFIG_KCSAN))
> > > > > +        return true;
> > > > > +
> > > > >          return false;
> > > > >      }
> > > > Unfortunately this doesn't get us any further (I'd love though, it looks
> > > > way cleaner!)
> > > > 
> > > > I still see the KCSAN messages even on boot.
> > > 
> > > Ah, looks like the important bit for KCSAN is not the mapping mechanism,
> > > it's the actual compiler annotation for the read. So these virtio ring reads
> > > should all be annotated as READ_ONCE() to make sure KCSAN knows the read
> > > itself is atomic.
> > > 
> > > Alex
> > > 
> > so then:
> > 
> >         return vq->last_used_idx != virtio16_to_cpu(vq->vq.vdev,
> >                         READ_ONCE(vq->split.vring.used->idx));
> 
> 
> Yes, which is on the verge of getting unreadable.

Well we can wrap virtio16_to_cpu and READ_ONCE in a single macro
if you like.  VIRTIO16_READ ?


> I'll work with Johannes on
> v3. We'll play with ways to make it a bit more maintainable. Please discard
> the current patches for now.

Sure.

> 
> Alex
> 
> 
> 
> 
> Amazon Web Services Development Center Germany GmbH
> Tamara-Danz-Str. 13
> 10243 Berlin
> Geschaeftsfuehrung: Christof Hellmis, Andreas Stieger
> Eingetragen am Amtsgericht Charlottenburg unter HRB 257764 B
> Sitz: Berlin
> Ust-ID: DE 365 538 597