* [PATCH] mshv: Allow mappings that overlap in uaddr
@ 2025-11-04 22:18 Nuno Das Neves
2025-11-06 13:38 ` Michael Kelley
0 siblings, 1 reply; 3+ messages in thread
From: Nuno Das Neves @ 2025-11-04 22:18 UTC (permalink / raw)
To: linux-hyperv, linux-kernel, magnuskulke
Cc: kys, haiyangz, wei.liu, decui, longli, mhklinux, skinsburskii,
prapal, mrathor, muislam, Nuno Das Neves
From: Magnus Kulke <magnuskulke@linux.microsoft.com>
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 <magnuskulke@linux.microsoft.com>
Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
---
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;
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
^ permalink raw reply [flat|nested] 3+ messages in thread* RE: [PATCH] mshv: Allow mappings that overlap in uaddr
2025-11-04 22:18 [PATCH] mshv: Allow mappings that overlap in uaddr Nuno Das Neves
@ 2025-11-06 13:38 ` Michael Kelley
2025-11-06 18:40 ` Nuno Das Neves
0 siblings, 1 reply; 3+ messages in thread
From: Michael Kelley @ 2025-11-06 13:38 UTC (permalink / raw)
To: Nuno Das Neves, linux-hyperv, linux-kernel, magnuskulke
Cc: kys, haiyangz, wei.liu, decui, longli, skinsburskii, prapal,
mrathor, muislam
From: Nuno Das Neves <nunodasneves@linux.microsoft.com> 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 <magnuskulke@linux.microsoft.com>
> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
> ---
> 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?
>
> 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 <mhklinux@outlook.com>
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] mshv: Allow mappings that overlap in uaddr
2025-11-06 13:38 ` Michael Kelley
@ 2025-11-06 18:40 ` Nuno Das Neves
0 siblings, 0 replies; 3+ messages in thread
From: Nuno Das Neves @ 2025-11-06 18:40 UTC (permalink / raw)
To: Michael Kelley, linux-hyperv, linux-kernel, magnuskulke
Cc: kys, haiyangz, wei.liu, decui, longli, skinsburskii, prapal,
mrathor, muislam
On 11/6/2025 5:38 AM, Michael Kelley wrote:
> From: Nuno Das Neves <nunodasneves@linux.microsoft.com> 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 <magnuskulke@linux.microsoft.com>
>> Signed-off-by: Nuno Das Neves <nunodasneves@linux.microsoft.com>
>> ---
>> 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 <mhklinux@outlook.com>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2025-11-06 18:41 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-11-04 22:18 [PATCH] mshv: Allow mappings that overlap in uaddr Nuno Das Neves
2025-11-06 13:38 ` Michael Kelley
2025-11-06 18:40 ` Nuno Das Neves
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®