Re: [PATCH v2] Drivers: hv: mshv: fix integer overflow in memory region overlap check

[Stanislav Kinsburskii]

On Sat, Mar 28, 2026 at 05:18:45PM +0800, Junrui Luo wrote:
> mshv_partition_create_region() computes mem->guest_pfn + nr_pages to
> check for overlapping regions without verifying u64 wraparound. A
> sufficiently large guest_pfn can cause the addition to overflow,
> bypassing the overlap check and allowing creation of regions that wrap
> around the address space.
> 
> Fix by using check_add_overflow() to reject such regions early, and
> validate that the region end does not exceed MAX_PHYSMEM_BITS. These
> checks also protect downstream callers that compute start_gfn +
> nr_pages on stored regions without overflow guards.
> 
> Fixes: 621191d709b1 ("Drivers: hv: Introduce mshv_root module to expose /dev/mshv to VMMs")
> Reported-by: Yuhao Jiang <danisjiang@xxxxxxxxx>
> Suggested-by: Roman Kisel <romank@xxxxxxxxxxxxxxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Junrui Luo <moonafterrain@xxxxxxxxxxx>
> ---
> Changes in v2:
> - Add a maximum check suggested by Roman Kisel
> - Link to v1: https://lore.kernel.org/all/SYBPR01MB7881689C0F58149DD986A6D1AF49A@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx/
> ---
>  drivers/hv/mshv_root_main.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/hv/mshv_root_main.c b/drivers/hv/mshv_root_main.c
> index 6f42423f7faa..32826247dbce 100644
> --- a/drivers/hv/mshv_root_main.c
> +++ b/drivers/hv/mshv_root_main.c
> @@ -1174,11 +1174,20 @@ static int mshv_partition_create_region(struct mshv_partition *partition,
>  {
>  	struct mshv_mem_region *rg;
>  	u64 nr_pages = HVPFN_DOWN(mem->size);
> +	u64 new_region_end;
> +
> +	/* Reject regions whose end address would wrap around */
> +	if (check_add_overflow(mem->guest_pfn, nr_pages, &new_region_end))
> +		return -EOVERFLOW;
> +
> +	/* Reject regions beyond the maximum physical address */

nit: both comments are redundant - the meaning is clear from the code
itself.

> +	if (new_region_end > HVPFN_DOWN(1ULL << MAX_PHYSMEM_BITS))

This maximum value check bugs me a bit.

First of all, why does it matter what is the region end? Potentially, there can be
regions not backed by host address space (leave alone host RAM), so why
intropducing this limitation?

Second, this check takes a host-specific constant (MAX_PHYSMEM_BITS) and rounds it down
to hypervisor-specific units which may not be aligned with the host page
size. Should this be host pages instead?

Thanks,
Stanislav

> +		return -EINVAL;
>  
>  	/* Reject overlapping regions */
>  	spin_lock(&partition->pt_mem_regions_lock);
>  	hlist_for_each_entry(rg, &partition->pt_mem_regions, hnode) {
> -		if (mem->guest_pfn + nr_pages <= rg->start_gfn ||
> +		if (new_region_end <= rg->start_gfn ||
>  		    rg->start_gfn + rg->nr_pages <= mem->guest_pfn)
>  			continue;
>  		spin_unlock(&partition->pt_mem_regions_lock);
> 
> ---
> base-commit: c369299895a591d96745d6492d4888259b004a9e
> change-id: 20260328-fixes-0296eb3dbb52
> 
> Best regards,
> -- 
> Junrui Luo <moonafterrain@xxxxxxxxxxx>