Hi Stephen, Thank you for flagging the issue. I have submitted v11 of the patch-set and the issue has been addressed.
Regards, Pravin Internal Use - Confidential > -----Original Message----- > From: Stephen Hemminger <[email protected]> > Sent: Monday, May 4, 2026 9:01 AM > To: Bathija, Pravin <[email protected]> > Cc: [email protected]; [email protected]; [email protected]; > [email protected] > Subject: Re: [PATCH v10 4/5] vhost_user: Function defs for add/rem mem > regions > > > [EXTERNAL EMAIL] > > On Tue, 7 Apr 2026 22:42:04 +0000 > <[email protected]> wrote: > > > From: Pravin M Bathija <[email protected]> > > > > These changes cover the function definition for add/remove memory > > region calls which are invoked on receiving vhost user message from > > vhost user front-end (e.g. Qemu). In our case, in addition to testing > > with qemu front-end, the testing has also been performed with libblkio > > front-end and spdk/dpdk back-end. We did I/O using libblkio based > > device driver, to spdk based drives. There are also changes for > > set_mem_table and new definition for get memory slots. Our changes > > optimize the set memory table call to use common support functions. > > Message get memory slots is how the vhost-user front-end queries the > > vhost-user back-end about the number of memory slots available to be > > registered by the back-end. In addition support function to invalidate > > vring is also defined which is used in add/remove memory region functions. > > > > Signed-off-by: Pravin M Bathija <[email protected]> > > --- > > The rest of the series review cleanly but AI flagged one issue. > The initial description was a mess. Asked it to be clearer. > > ## Review: PATCH v10 4/5 — vhost_user: Function defs for add/rem mem > regions > > **Warning: incomplete cleanup in `vhost_user_add_mem_reg()` mmap-failure > path.** > > What is wrong: when `vhost_user_mmap_region()` returns -1, the handler only > closes `reg->fd`. But `vhost_user_mmap_region()` can fail *after* setting > `reg- > >mmap_addr`/`mmap_size`/`host_user_addr` and after `add_guest_pages()` > has appended entries (the `add_one_guest_page()` realloc path). > > Impact: leaked mmap and stale `dev->guest_pages` entries. Since `nregions` is > not incremented, the next ADD_MEM_REG reuses the same slot and overwrites > `mmap_addr`, so `free_all_mem_regions()` can no longer reach the original > mapping. > > Fix: when `reg->mmap_addr` is set, take the `free_new_region` path > (`remove_guest_pages()` + `free_mem_region(reg)`) instead of just `close(reg- > >fd)`. Keep the explicit `close()` for the case where `mmap()` itself failed > >before > `mmap_addr` was assigned.
