linux-kernel - Re: [PATCH v9 5/6] vduse: add vq group asid support

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 for Android: free password hash cracker in your pocket

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

Message-ID: <CAJaqyWeAoVzmWaAwcK9kjj=3VuRmYc1fco7a1ausXB=z-0oK=w@mail.gmail.com>
Date: Wed, 19 Nov 2025 11:38:32 +0100
From: Eugenio Perez Martin <eperezma@...hat.com>
To: "Michael S. Tsirkin" <mst@...hat.com>
Cc: Jason Wang <jasowang@...hat.com>, Laurent Vivier <lvivier@...hat.com>, 
	virtualization@...ts.linux.dev, Maxime Coquelin <mcoqueli@...hat.com>, 
	Cindy Lu <lulu@...hat.com>, linux-kernel@...r.kernel.org, 
	Yongji Xie <xieyongji@...edance.com>, Xuan Zhuo <xuanzhuo@...ux.alibaba.com>, 
	Stefano Garzarella <sgarzare@...hat.com>
Subject: Re: [PATCH v9 5/6] vduse: add vq group asid support

On Wed, Nov 19, 2025 at 10:32 AM Michael S. Tsirkin <mst@...hat.com> wrote:
>
> On Wed, Nov 19, 2025 at 10:26:43AM +0100, Eugenio Perez Martin wrote:
> > > But this is not the logic that is
> > > implemented in this patch as there's no synchronize_rcu() in the
> > > vduse_set_group_asid_nomsg().
> >
> > We only set the pointer on the writer's side, we do nothing like
> > freeing resources. Should we set the pointer before or after
> > syncrhonize_rcu()?
>
> synchronize_rcu is called after writer makes it's changes.
>
> > What do we need to do on the other side of
> > syncrhonize_rcu()?
>
> Presumably, return so the caller knows the as has been updated.
>

I'm happy to add the syncrhonize_rcu() just in case, but the caller of
vduse_set_group_asid_nomsg does not need to know that the reader has
been updated.

The first caller is vduse_dev_reset which has its own way to avoid
being called while vqs are being processed. In particular, it reset
their addresses which is way more dangerous. I could call it with
dev->rwsem down though.

The second one is set_group_asid vdpa callback which is called from
the ioctl itself.

Moreover, rcu_assign_pointer is WRITE_ONCE by itself so we know all
the readers will get the new value after it. So what's the value of
explicitly waiting for all the readers to finalize their DMA operation?

I'd understand if we need to do modifications or memory management in
the now unused ASID, but that's not the case here.

> However, user-triggerable synchronize_rcu() is almost always a bug.
>
> If that's what is going on, you want srcu.
>

I did not know about it, thanks! but I think all the code that can
sleep is out of the RCU critical sections now, isn't it?