Re: [PATCH V3 10/11] mshv: Populate mmio mappings for PCI passthru

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 5 potential issue(s) to consider:
- [Critical] Unvalidated physical memory mapping and integer overflow allow mapping arbitrary host RAM.
- [Critical] Missing MMU notifier registration and TOCTOU race in MMIO fault handling.
- [High] Architecture page size mismatch corrupts physical address mappings on ARM64.
- [High] Hardcoded write permissions bypass host read-only restrictions for MMIO regions.
- [Medium] Uninitialized stack variable `pfnmap_args`.
--

Note: The format of this report is altered to provide a free-form summary of
the findings, rather than using traditional inline diff quotes, to adhere to
recitation restrictions.

commit f5a60b27c18f019e0a418a3f28dcff85e1128ed4
Author: Mukesh R <mrathor@linux.microsoft.com>
Subject: mshv: Populate mmio mappings for PCI passthru

This commit adds support for mapping mmio memory to the guest upon receiving
an unmapped guest physical address intercept. It looks up the physical frame
number from the driver and asks the hypervisor to establish the map.

While reviewing this code, a few questions came up regarding the handling of
the memory mappings.

When the memory region is created from the user memory structure, it appears
only the start address is validated against the virtual memory area. The page
count is taken directly from user input. Later, in the intercept handler, a
signed 32-bit integer is used to loop over the 64-bit page count. If a user
provides an excessively large size, could the loop counter overflow to a
negative value and prematurely terminate the validation loop? Would this
bypass the ram checks and allow mapping arbitrary host memory into the
guest, while also potentially causing a lockup without a scheduling point?

Is there a race condition when fetching the physical frame number? The memory
map read lock is dropped after resolving the address, and the hypercall is
made without holding it. Since there does not seem to be a memory management
unit interval notifier sequence check here, what happens if the host virtual
memory area is unmapped or modified concurrently? Could the guest retain
access to stale or reallocated host memory?

Does the physical frame number calculation work correctly on architectures
with varying page sizes? On arm64, the kernel page size might be 64KB,
meaning the value returned by the pfnmap functions is based on 64KB units.
However, the handler subtracts a 4KB-based offset from it. Will this
result in an incorrect base physical address being passed to the
hypervisor?

Will this logic handle read-only device memory properly? The code
unconditionally requests a write fault resolution and then hardcodes the
writable flag for the hypervisor map call. If userspace maps a read-only
memory region, will the fault fixup fail? If it does succeed, would the
guest be granted write access to a read-only region?

Does the pfnmap arguments structure need zero-initialization? The structure
is declared on the stack and only the virtual memory area and address
fields are explicitly populated. Could uninitialized padding or other
structure members contain stack garbage that might affect the pfnmap
operations?

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