On Tue, Oct 20, 2026 at 8:31 AM Michael S. Tsirkin <[email protected]> wrote:
>
> On Mon, Oct 19, 2026 at 03:14:46PM +0800, Wang Haoran wrote:
> > Eugenio previously offered some additional suggestions in reply to
> > my patch email. Should these also be addressed?
>
> The following seems to work for me. Testing would be appreciated.
>
> From: Michael S. Tsirkin <[email protected]>
> Subject: [PATCH v2] vringh: use get_user/put_user in the use_va path
>
> getu16_iotlb()/putu16_iotlb() pass the address produced by
> iotlb_translate() straight to __get_user()/__put_user(),
> without access_ok checked anywhere. Not good.
>
> Switch to get_user()/put_user(), which are always
> correct, and safer.
>
> But the fact is that accessing user memory like this
> will sometimes (rarely) race with page being swapped
> out, trigger EFAULT, and it should be retried
> from a worker, and vdpa_sim currently does not do this.
>
> Fixes: 42823a871fd4 ("vringh: support VA with iotlb")
> Cc: Stefano Garzarella <[email protected]>
> Signed-off-by: Michael S. Tsirkin <[email protected]>
>
> ---
>
> Notes (changelog):
> v1->v2:
> Use get_user()/put_user() wrapped in pagefault_disable() instead of
> access_ok() followed by __get_user()/__put_user(); the helpers are
> reached from BH-disabled sections, and __get_user() is not a valid
> way around that (David Laight, Linus Torvalds)
>
> drivers/vdpa/vdpa_sim/vdpa_sim_blk.c | 2 ++
> drivers/vdpa/vdpa_sim/vdpa_sim_net.c | 2 ++
> drivers/vhost/vringh.c | 4 ++--
> 3 files changed, 6 insertions(+), 2 deletions(-)
>
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> index 76dd5b0828d7..325ec8f0ab85 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_blk.c
> @@ -347,8 +347,10 @@ static void vdpasim_blk_work(struct vdpasim *vdpasim)
> smp_wmb();
>
> local_bh_disable();
> + pagefault_disable();
> if (vringh_need_notify_iotlb(&vq->vring) > 0)
> vringh_notify(&vq->vring);
> + pagefault_enable();
> local_bh_enable();
>
> if (++reqs > 4) {
> diff --git a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> index a6514b5ccd86..964cdf422467 100644
> --- a/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> +++ b/drivers/vdpa/vdpa_sim/vdpa_sim_net.c
> @@ -77,8 +77,10 @@ static void vdpasim_net_complete(struct vdpasim_virtqueue
> *vq, size_t len)
> smp_wmb();
>
> local_bh_disable();
> + pagefault_disable();
> if (vringh_need_notify_iotlb(&vq->vring) > 0)
> vringh_notify(&vq->vring);
> + pagefault_enable();
> local_bh_enable();
> }
>
Why not move the pagefault disable_enable to wrap just the accesses?
Future vdpa drivers could use va and we might miss the call. Also, the
reverse is also true, vdpa_sim could be configured to not use it, even
if it is not a big waste.
> diff --git a/drivers/vhost/vringh.c b/drivers/vhost/vringh.c
> index 9066f9f12ba1..74836eb60c7a 100644
> --- a/drivers/vhost/vringh.c
> +++ b/drivers/vhost/vringh.c
> @@ -1213,7 +1213,7 @@ static inline int getu16_iotlb(const struct vringh *vrh,
> return ret;
>
> if (vrh->use_va) {
> - ret = __get_user(tmp, (__virtio16 __user
> *)ivec.iov.iovec[0].iov_base);
> + ret = get_user(tmp, (__virtio16 __user
> *)ivec.iov.iovec[0].iov_base);
> if (ret)
> return ret;
> } else {
> @@ -1251,7 +1251,7 @@ static inline int putu16_iotlb(const struct vringh *vrh,
> tmp = cpu_to_vringh16(vrh, val);
>
> if (vrh->use_va) {
> - ret = __put_user(tmp, (__virtio16 __user
> *)ivec.iov.iovec[0].iov_base);
> + ret = put_user(tmp, (__virtio16 __user
> *)ivec.iov.iovec[0].iov_base);
> if (ret)
> return ret;
> } else {
>
