On 11/6/2025 5:38 AM, Michael Kelley wrote:
> From: Nuno Das Neves <[email protected]> Sent: Tuesday,
> November 4, 2025 2:19 PM
>>
>> Currently the MSHV driver rejects mappings that would overlap in
>> userspace.
>>
>> Some VMMs require the same memory to be mapped to different parts of
>> the guest's address space, and so working around this restriction is
>> difficult.
>>
>> The hypervisor itself doesn't prohibit mappings that overlap in uaddr,
>> (really in SPA: system physical addresses), so supporting this in the
>> driver doesn't require any extra work, only the checks need to be
>> removed.
>>
>> Since no userspace code up until has been able to overlap regions in
>> userspace, relaxing this constraint can't break any existing code.
>>
>> Signed-off-by: Magnus Kulke <[email protected]>
>> Signed-off-by: Nuno Das Neves <[email protected]>
>> ---
>> drivers/hv/mshv_root_main.c | 19 +------------------
>> include/uapi/linux/mshv.h | 2 +-
>> 2 files changed, 2 insertions(+), 19 deletions(-)
>>
>> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
>> index 814465a0912d..e5da5f2ab6f7 100644
>> --- a/drivers/hv/mshv_root_main.c
>> +++ b/drivers/hv/mshv_root_main.c
>> @@ -1206,21 +1206,6 @@ mshv_partition_region_by_gfn(struct mshv_partition
>> *partition, u64 gfn)
>> return NULL;
>> }
>>
>> -static struct mshv_mem_region *
>> -mshv_partition_region_by_uaddr(struct mshv_partition *partition, u64 uaddr)
>> -{
>> - struct mshv_mem_region *region;
>> -
>> - hlist_for_each_entry(region, &partition->pt_mem_regions, hnode) {
>> - if (uaddr >= region->start_uaddr &&
>> - uaddr < region->start_uaddr +
>> - (region->nr_pages << HV_HYP_PAGE_SHIFT))
>> - return region;
>> - }
>> -
>> - return NULL;
>> -}
>> -
>> /*
>> * NB: caller checks and makes sure mem->size is page aligned
>> * Returns: 0 with regionpp updated on success, or -errno
>> @@ -1235,9 +1220,7 @@ static int mshv_partition_create_region(struct
>> mshv_partition *partition,
>>
>> /* Reject overlapping regions */
>> if (mshv_partition_region_by_gfn(partition, mem->guest_pfn) ||
>> - mshv_partition_region_by_gfn(partition, mem->guest_pfn + nr_pages -
>> 1) ||
>> - mshv_partition_region_by_uaddr(partition, mem->userspace_addr) ||
>> - mshv_partition_region_by_uaddr(partition, mem->userspace_addr +
>> mem->size - 1))
>> + mshv_partition_region_by_gfn(partition, mem->guest_pfn + nr_pages -
>> 1))
>> return -EEXIST;
>
> This existing code (and after this patch) checks for overlap by seeing if the
> requested starting and ending GFNs are already in some existing region. But
> is this really sufficient to detect overlap? Consider this example:
>
> 1. Three regions exist covering these GFNs respectively: 100 thru 199,
> 300 thru 399, and 500 thru 599.
> 2. A request is made to create a new region for GFNs 250 thru 449.
>
> This new request would pass the check, but would still overlap. Or is there
> something that prevents this scenario?
>
The logic appears wrong to me. I will create a patch to fix it, and amend this
patch to work with that new logic since it will look a little different. I'll
post the fix + v2 of this patch as a series.
Thanks
Nuno
>>
>> region = vzalloc(sizeof(*region) + sizeof(struct page *) * nr_pages);
>> diff --git a/include/uapi/linux/mshv.h b/include/uapi/linux/mshv.h
>> index 9091946cba23..b10c8d1cb2ad 100644
>> --- a/include/uapi/linux/mshv.h
>> +++ b/include/uapi/linux/mshv.h
>> @@ -123,7 +123,7 @@ enum {
>> * @rsvd: MBZ
>> *
>> * Map or unmap a region of userspace memory to Guest Physical Addresses
>> (GPA).
>> - * Mappings can't overlap in GPA space or userspace.
>> + * Mappings can't overlap in GPA space.
>> * To unmap, these fields must match an existing mapping.
>> */
>> struct mshv_user_mem_region {
>> --
>> 2.34.1
>
> I've given my Reviewed-by: narrowly for this patch, since it appears to be
> correct for what it does. But if the approach for detecting overlap really
> is faulty, an additional patch is needed that might supersede this one.
>
> Reviewed-by: Michael Kelley <[email protected]>
