On Mon, May 11, 2026 at 02:26:45PM +0000, Anirudh Rayabharam wrote:
> On Thu, May 07, 2026 at 03:44:32PM +0000, Stanislav Kinsburskii wrote:
> > mshv_partition_ioctl_create_vp() called anon_inode_getfd() before
> > publishing the new VP into partition->pt_vp_array. anon_inode_getfd()
> > includes fd_install(), so the fd was live in current->files before the
> > publish ran.
> >
> > A concurrent MSHV_RUN_VP ioctl on that fd does not serialise against the
> > in-progress MSHV_CREATE_VP — it takes vp->vp_mutex, not the partition
> > mutex. Once the VP starts running and traps, mshv_intercept_isr() can look
> > up partition->pt_vp_array[vp_index] and observe NULL, silently dropping the
> > intercept message.
> >
> > Split the fd creation: reserve an fd with get_unused_fd_flags(), create the
> > file with anon_inode_getfile(), publish the VP via smp_store_release(), and
> > finally call fd_install() as the userspace-visibility commit point.
> >
> > Fixes: 621191d709b14 ("Drivers: hv: Introduce mshv_root module to expose
> > /dev/mshv to VMMs")
> > Signed-off-by: Stanislav Kinsburskii <[email protected]>
> > ---
> > drivers/hv/mshv_root_main.c | 29 ++++++++++++++++++++++-------
> > 1 file changed, 22 insertions(+), 7 deletions(-)
> >
> > diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> > index e32f6e0f9f637..1c18d1c1f7947 100644
> > --- a/drivers/hv/mshv_root_main.c
> > +++ b/drivers/hv/mshv_root_main.c
> > @@ -1142,6 +1142,8 @@ mshv_partition_ioctl_create_vp(struct mshv_partition
> > *partition,
> > struct mshv_vp *vp;
> > struct page *intercept_msg_page, *register_page, *ghcb_page;
> > struct hv_stats_page *stats_pages[2];
> > + struct file *file;
> > + int fd;
> > long ret;
> >
> > if (copy_from_user(&args, arg, sizeof(args)))
> > @@ -1214,14 +1216,18 @@ mshv_partition_ioctl_create_vp(struct
> > mshv_partition *partition,
> > if (ret)
> > goto put_partition;
> >
> > - /*
> > - * Keep anon_inode_getfd last: it installs fd in the file struct and
> > - * thus makes the state accessible in user space.
> > - */
> > - ret = anon_inode_getfd("mshv_vp", &mshv_vp_fops, vp,
> > - O_RDWR | O_CLOEXEC);
>
> Why not just move this anon_inode_getfd() after the smp_store_release()
> call?
>
Because anon_inode_getfd() can still fail at the fd-table step after the
VP is already visible to lockless ISR readers, and unpublishing it would
require a grace-period wait under pt_mutex while ISRs may have already
observed and acted on the doomed VP. The split form keeps every failable
step before the publish, so failure paths stay simple and the publish
itself cannot fail.
Thanks,
Stanislav
> Thanks,
> Anirudh.
