> -----Original Message----- > From: Naman Gulati <[email protected]> > Sent: 23 October 2026 22:42 > To: Thanos Makatos <[email protected]> > Cc: [email protected]; John Levon <[email protected]>; Cédric > Le Goater <[email protected]>; Alex Williamson <[email protected]>; > Pierrick Bouvier <[email protected]>; Matt Evans > <[email protected]> > Subject: Re: [PATCH v5 4/4] vfio-user: support multiple FDs for sparse > mmap regions > > !-------------------------------------------------------------------| > CAUTION: External Email > > |-------------------------------------------------------------------! > > On Tue, Oct 20, 2026 at 2:27 PM Thanos Makatos > <[email protected]> wrote: > > > > > > > > > -----Original Message----- > > > From: [email protected] > <qemu- > > > [email protected]> On Behalf Of > > > Naman Gulati > > > Sent: 14 October 2026 22:11 > > > To: [email protected] > > > Cc: John Levon <[email protected]>; Thanos Makatos > > > <[email protected]>; Cédric Le Goater <[email protected]>; > Alex > > > Williamson <[email protected]>; Pierrick Bouvier > > > <[email protected]>; Matt Evans <[email protected]>; > > > Naman Gulati <[email protected]> > > > Subject: [PATCH v5 4/4] vfio-user: support multiple FDs for sparse > > > mmap regions > > > > > > !----------------------------------------------------------------- > --| > > > CAUTION: External Email > > > > > > |----------------------------------------------------------------- > --! > > > > > > Add client support for capability > VFIO_REGION_INFO_CAP_SPARSE_MMAP_FDS > > > (capability ID 0x8001) in vfio-user, and advertise support via the > > > "sparse_mmap_fds" capability in VFIO_USER_VERSION. > > > > > > Update vfio_user_device_io_get_region_info() to receive multiple > file > > > descriptors passed in SCM_RIGHTS ancillary data for a region, > store > > > them > > > in VFIOUserProxy, and implement setup_sparse_mmaps in > > > vfio_user_device_io_ops_sock to validate and populate per-area > file > > > descriptors and fd_offset values. > > > > > > Signed-off-by: Naman Gulati <[email protected]> > > > --- > > > hw/vfio-user/protocol.h | 35 ++++++++++-- > > > hw/vfio-user/proxy.h | 2 + > > > hw/vfio-user/device.c | 121 > +++++++++++++++++++++++++++++++++++++-- > > > - > > > hw/vfio-user/proxy.c | 52 +++++++++++++++-- > > > 4 files changed, 193 insertions(+), 17 deletions(-) > > > > > > diff --git a/hw/vfio-user/protocol.h b/hw/vfio-user/protocol.h > > > index c478d1353f..b823689254 100644 > > > --- a/hw/vfio-user/protocol.h > > > +++ b/hw/vfio-user/protocol.h > > > @@ -15,6 +15,8 @@ > > > * SPDX-License-Identifier: GPL-2.0-or-later > > > */ > > > > > > +#define VFIO_USER_MAX_REGIONS 100 > > > + > > > typedef struct { > > > uint16_t id; > > > uint16_t command; > > > @@ -69,12 +71,13 @@ typedef struct { > > > #define VFIO_USER_CAP "capabilities" > > > > > > /* "capabilities" members */ > > > -#define VFIO_USER_CAP_MAX_FDS "max_msg_fds" > > > -#define VFIO_USER_CAP_MAX_XFER "max_data_xfer_size" > > > -#define VFIO_USER_CAP_PGSIZES "pgsizes" > > > -#define VFIO_USER_CAP_MAP_MAX "max_dma_maps" > > > -#define VFIO_USER_CAP_MIGR "migration" > > > -#define VFIO_USER_CAP_MULTI "write_multiple" > > > +#define VFIO_USER_CAP_MAX_FDS "max_msg_fds" > > > +#define VFIO_USER_CAP_MAX_XFER "max_data_xfer_size" > > > +#define VFIO_USER_CAP_PGSIZES "pgsizes" > > > +#define VFIO_USER_CAP_MAP_MAX "max_dma_maps" > > > +#define VFIO_USER_CAP_MIGR "migration" > > > +#define VFIO_USER_CAP_MULTI "write_multiple" > > > +#define VFIO_USER_CAP_SPARSE_MMAP_FDS "sparse_mmap_fds" > > > > > > /* "migration" members */ > > > #define VFIO_USER_CAP_PGSIZE "pgsize" > > > @@ -166,6 +169,26 @@ typedef struct { > > > uint64_t offset; > > > } VFIOUserRegionInfo; > > > > > > +/* > > > + * VFIO_REGION_INFO_CAP_SPARSE_MMAP_FDS > > > + */ > > > +#define VFIO_REGION_INFO_CAP_SPARSE_MMAP_FDS 0x8001 > > > + > > > +struct vfio_region_sparse_mmap_fd_area { > > > + uint64_t offset; > > > + uint64_t fd_offset; > > > + uint64_t size; > > > + uint32_t fd_index; > > > + uint32_t pad; > > > +}; > > > + > > > +struct vfio_region_info_cap_sparse_mmap_fds { > > > + struct vfio_info_cap_header header; > > > + uint32_t nr_areas; > > > + uint32_t reserved; > > > + struct vfio_region_sparse_mmap_fd_area areas[]; > > > +}; > > > + > > > /* > > > * VFIO_USER_DEVICE_GET_IRQ_INFO > > > * imported from struct vfio_irq_info > > > diff --git a/hw/vfio-user/proxy.h b/hw/vfio-user/proxy.h > > > index 7b97460cc5..4b9834a436 100644 > > > --- a/hw/vfio-user/proxy.h > > > +++ b/hw/vfio-user/proxy.h > > > @@ -72,6 +72,7 @@ typedef struct VFIOUserProxy { > > > AioContext *ctx; > > > QEMUBH *req_bh; > > > bool async_ops; > > > + VFIOUserFDs *region_fds[VFIO_USER_MAX_REGIONS]; > > > > > > /* > > > * above only changed when BQL is held > > > @@ -111,6 +112,7 @@ bool vfio_user_validate_version(VFIOUserProxy > > > *proxy, Error **errp); > > > > > > VFIOUserFDs *vfio_user_getfds(int numfds); > > > void vfio_user_putfds(VFIOUserMsg *msg); > > > +void vfio_user_free_fds(VFIOUserFDs *fds); > > > > > > void vfio_user_disable_posted_writes(VFIOUserProxy *proxy); > > > > > > diff --git a/hw/vfio-user/device.c b/hw/vfio-user/device.c > > > index 1a01e748cd..4fbf536a04 100644 > > > --- a/hw/vfio-user/device.c > > > +++ b/hw/vfio-user/device.c > > > @@ -14,12 +14,14 @@ > > > > > > #include "hw/vfio-user/device.h" > > > #include "hw/vfio-user/trace.h" > > > +#include "hw/vfio/vfio-region.h" > > > +#include "hw/vfio/vfio-helpers.h" > > > +#include "hw/vfio/trace.h" > > > > > > /* > > > * These are to defend against a malign server trying > > > * to force us to run out of memory. > > > */ > > > -#define VFIO_USER_MAX_REGIONS 100 > > > #define VFIO_USER_MAX_IRQS 50 > > > > > > bool vfio_user_get_device_info(VFIOUserProxy *proxy, > > > @@ -209,14 +211,17 @@ static int > > > vfio_user_device_io_get_region_info(VFIODevice *vbasedev, > > > struct > > > vfio_region_info *info, > > > int *fd) > > > { > > > - VFIOUserFDs fds = { 0, 1, fd}; > > > - int ret; > > > + int fds[VFIO_USER_MAX_MAX_FDS]; > > > + VFIOUserFDs user_fds = { 0, VFIO_USER_MAX_MAX_FDS, fds }; > > > + int i, ret; > > > + > > > + *fd = -1; > > > > > > if (info->index > vbasedev->num_initial_regions) { > > > return -EINVAL; > > > } > > > > > > - ret = vfio_user_get_region_info(vbasedev->proxy, info, &fds); > > > + ret = vfio_user_get_region_info(vbasedev->proxy, info, > > > &user_fds); > > > if (ret) { > > > return ret; > > > } > > > @@ -225,10 +230,30 @@ static int > > > vfio_user_device_io_get_region_info(VFIODevice *vbasedev, > > > if ((info->flags & VFIO_REGION_INFO_FLAG_CAPS) && > > > (info->cap_offset < sizeof(*info) > > > || info->cap_offset + sizeof(struct > vfio_info_cap_header) > > > > info->argsz)) { > > > - return -EINVAL; > > > + ret = -EINVAL; > > > + } else if (vfio_get_region_info_cap(info, > > > + > > > VFIO_REGION_INFO_CAP_SPARSE_MMAP_FDS)) { > > > + if (user_fds.recv_fds > 0) { > > > + VFIOUserFDs *saved = > vfio_user_getfds(user_fds.recv_fds); > > > + > > > + saved->recv_fds = user_fds.recv_fds; > > > + memcpy(saved->fds, fds, user_fds.recv_fds * > sizeof(int)); > > > + vfio_user_free_fds(vbasedev->proxy->region_fds[info- > > > >index]); > > > + vbasedev->proxy->region_fds[info->index] = saved; > > > + return 0; > > > + } > > > + ret = -EINVAL; > > > + } else if (user_fds.recv_fds > 0) { > > > + *fd = fds[0]; > > > + fds[0] = -1; > > > } > > > > > > - return 0; > > > + for (i = 0; i < user_fds.recv_fds; i++) { > > > + if (fds[i] >= 0) { > > > + close(fds[i]); > > > + } > > > + } > > > + return ret; > > > } > > > > > > static int vfio_user_device_io_get_irq_info(VFIODevice *vbasedev, > > > @@ -518,12 +543,94 @@ static int > > > vfio_user_device_io_region_write(VFIODevice *vbasedev, uint8_t > index, > > > /* > > > * Socket-based io_ops > > > */ > > > +static int vfio_user_device_io_setup_sparse_mmaps(VFIORegion > *region, > > > + struct > > > vfio_region_info *info, > > > + Error **errp) > > > +{ > > > + struct vfio_info_cap_header *hdr; > > > + struct vfio_region_info_cap_sparse_mmap_fds *sparse_fds; > > > + VFIOUserFDs *user_fds; > > > + size_t cap_offset; > > > + int i; > > > + > > > + hdr = vfio_get_region_info_cap(info, > > > VFIO_REGION_INFO_CAP_SPARSE_MMAP_FDS); > > > + if (!hdr) { > > > + return vfio_default_setup_sparse_mmaps(region, info, > errp); > > > + } > > > + > > > + if (vfio_get_region_info_cap(info, > > > VFIO_REGION_INFO_CAP_SPARSE_MMAP)) { > > > + error_setg(errp, "%s: region %d cannot specify both " > > > + "SPARSE_MMAP and SPARSE_MMAP_FDS", __func__, > > > region->nr); > > > + return -EINVAL; > > > + } > > > + > > > + sparse_fds = container_of(hdr, struct > > > vfio_region_info_cap_sparse_mmap_fds, > > > + header); > > > + > > > + cap_offset = (uintptr_t)hdr - (uintptr_t)info; > > > + if (cap_offset > info->argsz || > > > + info->argsz - cap_offset < sizeof(*sparse_fds) || > > > + sparse_fds->nr_areas > > > > + (info->argsz - cap_offset - sizeof(*sparse_fds)) / > > > + sizeof(sparse_fds->areas[0])) { > > > + error_setg(errp, "%s: invalid nr_areas %u in region %d", > > > + __func__, sparse_fds->nr_areas, region->nr); > > > + return -EINVAL; > > > + } > > > + > > > + trace_vfio_region_sparse_mmap_header(region->vbasedev->name, > > > + region->nr, sparse_fds- > > > >nr_areas); > > > + > > > + user_fds = region->vbasedev->proxy->region_fds[region->nr]; > > > + if (!user_fds || user_fds->recv_fds > sparse_fds->nr_areas) { > > > + error_setg(errp, "%s: invalid fd count %d for %u sparse > mmap > > > areas " > > > + "in region %d", __func__, > > > + user_fds ? user_fds->recv_fds : 0, > > > + sparse_fds->nr_areas, region->nr); > > > + return -EINVAL; > > > + } > > > + > > > + region->mmaps = g_try_new0(VFIOMmap, sparse_fds->nr_areas); > > > > IMO we should impose a limit on sparse_fds->nr_areas, g_try_new0 > does prevent the server from crashing but a malicious client can > allocate too much memory. > > sparse_fds->nr_areas is already bounded by info->argsz via the check > just above: > > if (cap_offset > info->argsz || > info->argsz - cap_offset < sizeof(*sparse_fds) || > sparse_fds->nr_areas > > (info->argsz - cap_offset - sizeof(*sparse_fds)) / > sizeof(sparse_fds->areas[0])) { > error_setg(errp, "%s: invalid nr_areas %u in region %d", > __func__, sparse_fds->nr_areas, region->nr); > return -EINVAL; > } > > And in vfio_user_get_region_info(), info->argsz is capped by > proxy->max_xfer_size (at most VFIO_USER_DEF_MAX_XFER = 1 MiB), so > sparse_fds->nr_areas cannot exceed ~32K entries (the same bound that > applies to VFIO_REGION_INFO_CAP_SPARSE_MMAP in > vfio_region_setup_sparse_mmaps()). > > Does that existing bound look sufficient to you, or would you still > prefer an explicit smaller limit (e.g. a VFIO_USER_MAX_SPARSE_MMAPS > constant alongside VFIO_USER_MAX_REGIONS)?
That's fine, thanks. Reviewed-by: Thanos Makatos <[email protected]> > > > > > > + if (!region->mmaps) { > > > + error_setg(errp, "%s: failed to allocate %u mmaps for > region > > > %d", > > > + __func__, sparse_fds->nr_areas, region->nr); > > > + return -ENOMEM; > > > + } > > > + > > > + for (i = 0; i < sparse_fds->nr_areas; i++) { > > > + struct vfio_region_sparse_mmap_fd_area *area = > &sparse_fds- > > > >areas[i]; > > > + > > > + if (area->size == 0 || area->offset > region->size || > > > + area->size > region->size - area->offset || > > > + area->fd_index >= user_fds->recv_fds) { > > > + error_setg(errp, "%s: invalid sparse mmap area %d in > > > region %d " > > > + "(offset=0x%"PRIx64" size=0x%"PRIx64 > > > + " fd_offset=0x%"PRIx64" fd_index=%u)", > > > + __func__, i, region->nr, area->offset, > area- > > > >size, > > > + area->fd_offset, area->fd_index); > > > + g_free(region->mmaps); > > > + region->mmaps = NULL; > > > + return -EINVAL; > > > + } > > > + > > > + trace_vfio_region_sparse_mmap_entry(i, area->offset, > > > + area->offset + area- > >size > > > - 1); > > > + region->mmaps[i].offset = area->offset; > > > + region->mmaps[i].fd_offset = area->fd_offset; > > > + region->mmaps[i].size = area->size; > > > + region->mmaps[i].fd = user_fds->fds[area->fd_index]; > > > + } > > > + > > > + region->nr_mmaps = sparse_fds->nr_areas; > > > + return 0; > > > +} > > > + > > > VFIODeviceIOOps vfio_user_device_io_ops_sock = { > > > .device_feature = vfio_user_device_io_device_feature, > > > .get_region_info = vfio_user_device_io_get_region_info, > > > + .setup_sparse_mmaps = vfio_user_device_io_setup_sparse_mmaps, > > > .get_irq_info = vfio_user_device_io_get_irq_info, > > > .set_irqs = vfio_user_device_io_set_irqs, > > > .region_read = vfio_user_device_io_region_read, > > > .region_write = vfio_user_device_io_region_write, > > > - > > > }; > > > diff --git a/hw/vfio-user/proxy.c b/hw/vfio-user/proxy.c > > > index 197aee07bf..ba29085321 100644 > > > --- a/hw/vfio-user/proxy.c > > > +++ b/hw/vfio-user/proxy.c > > > @@ -363,6 +363,13 @@ static int vfio_user_recv_one(VFIOUserProxy > > > *proxy, Error **errp) > > > } > > > > > > memcpy(msg->fds->fds, fdp, numfds * sizeof(int)); > > > + } else if (msg->fds != NULL) { > > > + /* > > > + * recv_fds holds the capacity of msg->fds->fds on entry; > > > reset it > > > + * to 0 when the reply carries no FDs so the caller knows > > > none were > > > + * received. > > > + */ > > > + msg->fds->recv_fds = 0; > > > } > > > > > > /* > > > @@ -740,6 +747,11 @@ bool vfio_user_send_wait(VFIOUserProxy > *proxy, > > > VFIOUserHdr *hdr, > > > > > > qemu_mutex_unlock(&proxy->lock); > > > > > > + if (!ok && fds != NULL) { > > > + /* No reply was received; report 0 received FDs to the > > > caller. */ > > > + fds->recv_fds = 0; > > > + } > > > + > > > return ok; > > > } > > > > > > @@ -867,17 +879,30 @@ void vfio_user_send_error(VFIOUserProxy > *proxy, > > > VFIOUserHdr *hdr, int error) > > > } > > > > > > /* > > > - * Close FDs erroneously received in an incoming request. > > > + * Close and free received FDs. > > > */ > > > -void vfio_user_putfds(VFIOUserMsg *msg) > > > +void vfio_user_free_fds(VFIOUserFDs *fds) > > > { > > > - VFIOUserFDs *fds = msg->fds; > > > int i; > > > > > > + if (!fds) { > > > + return; > > > + } > > > + > > > for (i = 0; i < fds->recv_fds; i++) { > > > - close(fds->fds[i]); > > > + if (fds->fds[i] >= 0) { > > > + close(fds->fds[i]); > > > + } > > > } > > > g_free(fds); > > > +} > > > + > > > +/* > > > + * Close FDs erroneously received in an incoming request. > > > + */ > > > +void vfio_user_putfds(VFIOUserMsg *msg) > > > +{ > > > + vfio_user_free_fds(msg->fds); > > > msg->fds = NULL; > > > } > > > > > > @@ -967,6 +992,7 @@ void vfio_user_set_handler(VFIODevice > *vbasedev, > > > void vfio_user_disconnect(VFIOUserProxy *proxy) > > > { > > > VFIOUserMsg *r1, *r2; > > > + int i; > > > > > > qemu_mutex_lock(&proxy->lock); > > > > > > @@ -1027,6 +1053,10 @@ void vfio_user_disconnect(VFIOUserProxy > *proxy) > > > vfio_user_iothread = NULL; > > > } > > > > > > + for (i = 0; i < VFIO_USER_MAX_REGIONS; i++) { > > > + vfio_user_free_fds(proxy->region_fds[i]); > > > + } > > > + > > > g_free(proxy->sockname); > > > g_free(proxy); > > > } > > > @@ -1214,6 +1244,18 @@ static bool check_multi(VFIOUserProxy > *proxy, > > > QObject *qobj, Error **errp) > > > return true; > > > } > > > > > > +static bool check_sparse_mmap_fds(VFIOUserProxy *proxy, QObject > > > *qobj, > > > + Error **errp) > > > +{ > > > + QBool *qb = qobject_to(QBool, qobj); > > > + > > > + if (qb == NULL) { > > > + error_setg(errp, "malformed %s", > > > VFIO_USER_CAP_SPARSE_MMAP_FDS); > > > + return false; > > > + } > > > + return true; > > > +} > > > + > > > static struct cap_entry caps_cap[] = { > > > { VFIO_USER_CAP_MAX_FDS, check_max_fds }, > > > { VFIO_USER_CAP_MAX_XFER, check_max_xfer }, > > > @@ -1221,6 +1263,7 @@ static struct cap_entry caps_cap[] = { > > > { VFIO_USER_CAP_MAP_MAX, check_max_dma }, > > > { VFIO_USER_CAP_MIGR, check_migr }, > > > { VFIO_USER_CAP_MULTI, check_multi }, > > > + { VFIO_USER_CAP_SPARSE_MMAP_FDS, check_sparse_mmap_fds }, > > > { NULL } > > > }; > > > > > > @@ -1280,6 +1323,7 @@ static GString *caps_json(void) > > > qdict_put_int(capdict, VFIO_USER_CAP_PGSIZES, > > > VFIO_USER_DEF_PGSIZE); > > > qdict_put_int(capdict, VFIO_USER_CAP_MAP_MAX, > > > VFIO_USER_DEF_MAP_MAX); > > > qdict_put_bool(capdict, VFIO_USER_CAP_MULTI, true); > > > + qdict_put_bool(capdict, VFIO_USER_CAP_SPARSE_MMAP_FDS, true); > > > > > > qdict_put_obj(dict, VFIO_USER_CAP, QOBJECT(capdict)); > > > > > > -- > > > 2.56.0.385.gd3acb90ef8-goog > > > > >
