Re: [PATCH v2 3/3] vhost: Fix SHMEM_MAP transaction ordering and resulting deadlock

["Michael S. Tsirkin" <mst@redhat.com>]

On Fri, Jul 17, 2026 at 12:07:21PM +0200, Dorinda Bassey wrote:
> You're right, I will reorder it in v3. The fix came last because I only
> discovered the deadlock while testing the shmem/blob support,
> but it makes sense to have it first since patch 2 depends on it.

thanks. it needs a Fixes tag too.

> On Fri, Jul 17, 2026 at 12:00 PM Michael S. Tsirkin <mst@redhat.com> wrote:
> 
>     On Fri, Jul 17, 2026 at 11:39:25AM +0200, Dorinda Bassey wrote:
>     > Commit the memory region transaction before sending the SHMEM_MAP reply
>     > so the KVM memory slot exists before the guest can access the mapped
>     > region.
>     >
>     > This moves the commit to while the backend is still blocked waiting for
>     > the reply, which can trigger ADD_MEM_REG back to the backend and
>     > deadlock. Filter shmem mapping regions out of vhost_section() by
>     > checking the MR owner type to prevent this.
>     >
>     > The explicit transaction begin/commit wrapping is dropped since
>     > virtio_add_shmem_map() already commits via memory_region_add_subregion().
>     >
>     > Suggested-by: Albert Esteve <aesteve@redhat.com>
>     > Signed-off-by: Dorinda Bassey <dbassey@redhat.com>
> 
>     i do not get this ordering) of patches) why are there features and last
>     patch
>     is a bugfix? if this is a dependency it should come first.
> 
>     > ---
>     >  hw/virtio/vhost-user.c | 20 ++------------------
>     >  hw/virtio/vhost.c      | 10 ++++++++++
>     >  2 files changed, 12 insertions(+), 18 deletions(-)
>     >
>     > diff --git a/hw/virtio/vhost-user.c b/hw/virtio/vhost-user.c
>     > index 517cc4ca716..01468f7441a 100644
>     > --- a/hw/virtio/vhost-user.c
>     > +++ b/hw/virtio/vhost-user.c
>     > @@ -1977,8 +1977,6 @@ vhost_user_backend_handle_shmem_map(struct
>     vhost_dev *dev,
>     >          }
>     >      }
>     > 
>     > -    memory_region_transaction_begin();
>     > -
>     >      /* Create VirtioSharedMemoryMapping object */
>     >      VirtioSharedMemoryMapping *mapping =
>     virtio_shared_memory_mapping_new(
>     >          vu_mmap->shmid, fd, vu_mmap->fd_offset, vu_mmap->shm_offset,
>     > @@ -1986,7 +1984,7 @@ vhost_user_backend_handle_shmem_map(struct
>     vhost_dev *dev,
>     > 
>     >      if (!mapping) {
>     >          ret = -EFAULT;
>     > -        goto send_reply_commit;
>     > +        goto send_reply;
>     >      }
>     > 
>     >      /* Add the mapping to the shared memory region */
>     > @@ -1994,22 +1992,8 @@ vhost_user_backend_handle_shmem_map(struct
>     vhost_dev *dev,
>     >          error_report("Failed to add shared memory mapping");
>     >          object_unref(OBJECT(mapping));
>     >          ret = -EFAULT;
>     > -        goto send_reply_commit;
>     > -    }
>     > -
>     > -send_reply_commit:
>     > -    /* Send reply and commit after transaction started */
>     > -    if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
>     > -        payload->u64 = !!ret;
>     > -        hdr->size = sizeof(payload->u64);
>     > -        if (!vhost_user_send_resp(ioc, hdr, payload, &local_err)) {
>     > -            error_report_err(local_err);
>     > -            memory_region_transaction_commit();
>     > -            return -EFAULT;
>     > -        }
>     > +        goto send_reply;
>     >      }
>     > -    memory_region_transaction_commit();
>     > -    return 0;
>     > 
>     >  send_reply:
>     >      if (hdr->flags & VHOST_USER_NEED_REPLY_MASK) {
>     > diff --git a/hw/virtio/vhost.c b/hw/virtio/vhost.c
>     > index af41841b529..e5c219be095 100644
>     > --- a/hw/virtio/vhost.c
>     > +++ b/hw/virtio/vhost.c
>     > @@ -636,6 +636,16 @@ static bool vhost_section(struct vhost_dev *dev,
>     MemoryRegionSection *section)
>     >  {
>     >      MemoryRegion *mr = section->mr;
>     > 
>     > +    /*
>     > +     * Skip shmem mapping regions, they are managed via SHMEM_MAP/UNMAP.
>     > +     * Including them triggers ADD_MEM_REG during the SHMEM_MAP
>     transaction
>     > +     * commit, deadlocking the backend.
>     > +     */
>     > +    if (object_dynamic_cast(memory_region_owner(mr),
>     > +                            TYPE_VIRTIO_SHARED_MEMORY_MAPPING)) {
>     > +        return false;
>     > +    }
>     > +
>     >      if (memory_region_is_ram(mr) && !memory_region_is_rom(mr)) {
>     >          uint8_t dirty_mask = memory_region_get_dirty_log_mask(mr);
>     >          uint8_t handled_dirty;
>     > --
>     > 2.52.0
> 
>