Re: [PATCH] mshv: fix fd leak in mshv_ioctl_create_vtl()

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] The `mshv_vtl_fault` mmap fault handler fails to bounds-check the user-controlled `cpu` index against `nr_cpu_ids`.
--

--- Patch [1]: [PATCH] mshv: fix fd leak in mshv_ioctl_create_vtl() ---
commit 2e3734b84c858c82d0ec893b4137e370673f6928
Author: Yi Xie <[email protected]>

mshv: fix fd leak in mshv_ioctl_create_vtl()

This commit addresses a file descriptor leak in mshv_ioctl_create_vtl().
It ensures put_unused_fd() is called when anon_inode_getfile() fails,
freeing the allocated file descriptor before returning an error.

> diff --git a/drivers/hv/mshv_vtl_main.c b/drivers/hv/mshv_vtl_main.c
> index 0d3d4161974f8..897a41b08d026 100644
> --- a/drivers/hv/mshv_vtl_main.c
> +++ b/drivers/hv/mshv_vtl_main.c
> @@ -129,6 +129,7 @@ mshv_ioctl_create_vtl(void __user *user_arg, struct device *module_dev)
>  	file = anon_inode_getfile("mshv_vtl", &mshv_vtl_fops,
>  				  vtl, O_RDWR);
>  	if (IS_ERR(file)) {
> +		put_unused_fd(fd);
>  		kfree(vtl);
>  		return PTR_ERR(file);
>  	}

[Severity: Critical]
This is a pre-existing issue, but while reviewing this file I noticed an
insufficient bounds check in the mmap fault handler.

Does this missing validation allow an out-of-bounds pointer arithmetic
regression?

drivers/hv/mshv_vtl_main.c:mshv_vtl_fault() {
	struct page *page;
	int cpu = vmf->pgoff & MSHV_PG_OFF_CPU_MASK;
	int real_off = vmf->pgoff >> MSHV_REAL_OFF_SHIFT;

	if (!cpu_online(cpu))
		return VM_FAULT_SIGBUS;

A VMA with a large offset can cause vmf->pgoff to yield an arbitrary
cpu index. The only validation is cpu_online(cpu), which uses an
unbounds-checked test_bit() against cpu_online_mask.

If the out-of-bounds read hits a set bit, the check passes. Execution then
proceeds to mshv_vtl_cpu_run(cpu), calling per_cpu_ptr() which reads from
the __per_cpu_offset array out-of-bounds. 

The resulting garbage pointer is passed to virt_to_page(), and get_page()
incorrectly increments a reference count, potentially mapping arbitrary
physical memory into userspace.

Should this code check the cpu index against nr_cpu_ids before passing it
to cpu_online()?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=1