[PATCH] vhost: keep vring addresses stable while the backend is attached

[PATCH] vhost: keep vring addresses stable while the backend is attached

[Jia Jia]

vhost-scsi keeps the response and bounce-buffer iovecs until the backend
completes the command.  If userspace changes the vring addresses while the
command is in flight, the completion data is copied through the old iovecs
while vhost_add_used() updates the new used ring.  The guest then no longer
sees the completion.

Reject changes to the vring addresses while a backend is attached to the
queue.  Keep accepting the current addresses so userspace can update
VHOST_VRING_F_LOG or log_guest_addr without stopping the backend.  Queues
that have not been attached retain the existing setup behavior; userspace
must detach the backend before changing their addresses.

Fixes: 057cbf49a1f0 ("tcm_vhost: Initial merge for vhost level target fabric driver")
Signed-off-by: Jia Jia <physicalmtea@gmail.com>
---
 drivers/vhost/vhost.c | 11 +++++++++++
 1 file changed, 11 insertions(+)

diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
index 44cac11b68d2..074590838de5 100644
--- a/drivers/vhost/vhost.c
+++ b/drivers/vhost/vhost.c
@@ -2150,6 +2150,17 @@ static long vhost_vring_set_addr(struct vhost_dev *d,
 				a.flags & (0x1 << VHOST_VRING_F_LOG),
 				a.log_guest_addr))
 			return -EINVAL;
+
+		/*
+		 * Commands may retain iovecs derived from the current ring
+		 * addresses.  Keep the addresses unchanged while the backend
+		 * is attached.
+		 */
+		if ((vq->desc || vq->avail || vq->used) &&
+		    (vq->desc != (void __user *)(unsigned long)a.desc_user_addr ||
+		     vq->avail != (void __user *)(unsigned long)a.avail_user_addr ||
+		     vq->used != (void __user *)(unsigned long)a.used_user_addr))
+			return -EBUSY;
 	}
 
 	vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG));

Re: [PATCH] vhost: keep vring addresses stable while the backend is attached

[Michael S. Tsirkin]

On Sat, Sep 19, 2026 at 09:17:44PM +0800, Jia Jia wrote:
> vhost-scsi keeps the response and bounce-buffer iovecs until the backend
> completes the command.  If userspace changes the vring addresses while the
> command is in flight, the completion data is copied through the old iovecs
> while vhost_add_used() updates the new used ring.  The guest then no longer
> sees the completion.

I do not get what all this is trying to say. if you break it you get to
keep both pieces. when is this a real problem?

> 
> Reject changes to the vring addresses while a backend is attached to the
> queue.  Keep accepting the current addresses so userspace can update
> VHOST_VRING_F_LOG or log_guest_addr without stopping the backend.  Queues
> that have not been attached retain the existing setup behavior; userspace
> must detach the backend before changing their addresses.
> 
> Fixes: 057cbf49a1f0 ("tcm_vhost: Initial merge for vhost level target fabric driver")
> Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> ---
>  drivers/vhost/vhost.c | 11 +++++++++++
>  1 file changed, 11 insertions(+)
> 
> diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> index 44cac11b68d2..074590838de5 100644
> --- a/drivers/vhost/vhost.c
> +++ b/drivers/vhost/vhost.c
> @@ -2150,6 +2150,17 @@ static long vhost_vring_set_addr(struct vhost_dev *d,
>  				a.flags & (0x1 << VHOST_VRING_F_LOG),
>  				a.log_guest_addr))
>  			return -EINVAL;
> +
> +		/*
> +		 * Commands may retain iovecs derived from the current ring
> +		 * addresses.  Keep the addresses unchanged while the backend
> +		 * is attached.
> +		 */
> +		if ((vq->desc || vq->avail || vq->used) &&
> +		    (vq->desc != (void __user *)(unsigned long)a.desc_user_addr ||
> +		     vq->avail != (void __user *)(unsigned long)a.avail_user_addr ||
> +		     vq->used != (void __user *)(unsigned long)a.used_user_addr))
> +			return -EBUSY;
>  	}
>  
>  	vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG));

Re: [PATCH] vhost: keep vring addresses stable while the backend is attached

[Jia Jia]

>
> On Sat, Sep 19, 2026 at 09:17:44PM +0800, Jia Jia wrote:
> > vhost-scsi keeps the response and bounce-buffer iovecs until the backend
> > completes the command.  If userspace changes the vring addresses while the
> > command is in flight, the completion data is copied through the old iovecs
> > while vhost_add_used() updates the new used ring.  The guest then no longer
> > sees the completion.
>
> I do not get what all this is trying to say. if you break it you get to
> keep both pieces. when is this a real problem?
>

Before running the test, I checked the virtio spec and vhost UAPI and
did not find an explicit rule
prohibiting changes to vring addresses while a backend is attached.
That is why I ran the directed test.
I see what you mean now. I'll drop this patch.

> >
> > Reject changes to the vring addresses while a backend is attached to the
> > queue.  Keep accepting the current addresses so userspace can update
> > VHOST_VRING_F_LOG or log_guest_addr without stopping the backend.  Queues
> > that have not been attached retain the existing setup behavior; userspace
> > must detach the backend before changing their addresses.
> >
> > Fixes: 057cbf49a1f0 ("tcm_vhost: Initial merge for vhost level target fabric driver")
> > Signed-off-by: Jia Jia <physicalmtea@gmail.com>
> > ---
> >  drivers/vhost/vhost.c | 11 +++++++++++
> >  1 file changed, 11 insertions(+)
> >
> > diff --git a/drivers/vhost/vhost.c b/drivers/vhost/vhost.c
> > index 44cac11b68d2..074590838de5 100644
> > --- a/drivers/vhost/vhost.c
> > +++ b/drivers/vhost/vhost.c
> > @@ -2150,6 +2150,17 @@ static long vhost_vring_set_addr(struct vhost_dev *d,
> >                               a.flags & (0x1 << VHOST_VRING_F_LOG),
> >                               a.log_guest_addr))
> >                       return -EINVAL;
> > +
> > +             /*
> > +              * Commands may retain iovecs derived from the current ring
> > +              * addresses.  Keep the addresses unchanged while the backend
> > +              * is attached.
> > +              */
> > +             if ((vq->desc || vq->avail || vq->used) &&
> > +                 (vq->desc != (void __user *)(unsigned long)a.desc_user_addr ||
> > +                  vq->avail != (void __user *)(unsigned long)a.avail_user_addr ||
> > +                  vq->used != (void __user *)(unsigned long)a.used_user_addr))
> > +                     return -EBUSY;
> >       }
> >
> >       vq->log_used = !!(a.flags & (0x1 << VHOST_VRING_F_LOG));
>