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

Reply via email to