Re: [PATCH V4 9/9] mshv: Disable movable regions upfront if device passthru

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Malicious userspace can bypass memory pinning enforcement by forcing copy_to_user() to fail.
- [Low] Module parameter `hv_memreg` is exposed as runtime-writable despite being designed as a boot parameter.
--

--- Patch [9]: [PATCH V4 9/9] mshv: Disable movable regions upfront if device passthru ---
commit c768ee85b3efb1c981444ab02e4e4d365693795e
Author: Mukesh R <mrathor@linux.microsoft.com>

mshv: Disable movable regions upfront if device passthru

If a VM is started with a device attached, the memory regions must be marked
non-movable since the device attach hypercall allows the use of SLAT for IOMMU.
This patch marks them non-movable upfront upon device passthru and adds a boot
parameter to allow a user to pin them manually if anticipated.

> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 58cf61cb227ba..b085fa13574f0 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -46,6 +46,10 @@ MODULE_DESCRIPTION("Microsoft Hyper-V root partition VMM interface /dev/mshv");
>  #define HV_VP_COUNTER_ROOT_DISPATCH_THREAD_BLOCKED 95
>  #endif
>  
> +static uint hv_memreg;  /* mshv_mem_region allocations type */
> +module_param(hv_memreg, uint, 0644);

[Severity: Low]
The commit message states this is a "boot parameter". Is it intentional
that this module parameter is created with 0644 permissions? This allows
root to change the allocation policy at runtime, whereas 0444 would
correctly enforce its boot-only nature.

> +MODULE_PARM_DESC(hv_memreg, "0: movable (default), 1: pinned");
> +
>  struct mshv_root mshv_root;

[ ... ]

> @@ -1801,6 +1811,9 @@ static long mshv_partition_ioctl_create_device(struct mshv_partition *partition,
>  	if (copy_to_user(uarg, &devargk, sizeof(devargk)))
>  		return -EFAULT;    /* cleanup in mshv_device_fop_release() */

[Severity: High]
If userspace passes a read-only memory buffer to the MSHV_CREATE_DEVICE ioctl,
copy_from_user() can succeed but copy_to_user() will fault and return -EFAULT.

Because the file descriptor has already been installed via anon_inode_getfd()
prior to the copy_to_user() call, the user can recover the functional device
FD via /proc/self/fd/ even after the ioctl returns an error.

Does this early return allow malicious userspace to bypass the memory pinning
enforcement by forcing copy_to_user() to fail, intentionally skipping the
partition->pt_regions_pinned = true assignment?

>  
> +	/* For now, just mark regions pinned if there is device passthru. */
> +	partition->pt_regions_pinned = true;
> +
>  	return 0;
>  
>  undo_out:

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260718021949.926306-1-mrathor@linux.microsoft.com?part=9