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

[vdso@mailbox.org]

> On 04/09/2026 8:06 PM PDT Junrui Luo <moonafterrain@outlook.com> wrote:
> 
>  
> On Thu, Apr 02, 2026 at 04:25:02PM -0700, Stanislav Kinsburskii wrote:
> > nit: both comments are redundant - the meaning is clear from the code
> > itself.
> 
> I will drop them in v3.
> 
> > 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?
>  
> This check was suggested by Roman in v1 review. Roman, could you
> share your thoughts on Stanislav's concerns? I'd like to align on whether an upper
> bound check is needed here.

Hey Junrui, Stanislav,

The idea was that there is a max PFN/GFN going over which is _architecturally_ impossible.
It might be ((1 << 52) - 1) << 12 or ((1 << 48) - 1) << 12 or similar depending on how
many bits are used for the guest phys. address (52, 48, 36) and page sizes. The gist is
that max is way below 0xFFFF_FFFF_FFFF_FFFF (1<<64 - 1) used for overflow checking in the
original v1 patch. Hence, instead of checking for overflow, one may check for that max
PFN/GFN catching way more bad input. Checking for that max also feels as more
domain-specific compared to the "generic" overflow check.

I went for the trade-off where the "impossible" PFN/GFN is computed simply as
(1 << max_possible_bits_in_the_address)/the_min_possible_page_size. Again, that's simple
and better than the oveflow check as it catches the region with PFNs/GFNs that just cannot
be used. I agree that may overshoot, and an even more specific check would have to fetch the
CPUID for the VM (or its ARM64 MMFR regs), etc. to get more specifics and a lower bound
on the bad GFN (but at the cost of more computation).

All in all, from the three options of (generic check for overflow, simple check
for arch bad PFNs/GFNs, an elaborated check with all specifics) I suggested the simple check.
Fast and still more useful than checking for overflow in my opinion.

Below I go into various details on "why impossible" and these 48 and 52 bits, and I most
likely will be re-iterating things that you know - just for the convenience of other
folks who don't play with MMU/EPT/etc often, and might find the topic interesting.
It may happen that I'll also learn something from your critique, thanks in advance :)

That max PFN/GFN is dictated by the maximum number of bits allowed to be used in
the physical address. Above that, an address would not be mapped or cached by
the machines that run the code in question. The page tables won't work as the number
of bits used for the physical address here is the sum of the bits used to index
into a page table + the offset bits. That also gives the number of bits in the virtual
address which the CPU needs to address the memory (again, in this modern case;
there were many years ago machines with 32 bit virt addresses and 36 bit phys
addresses used via PAE/AWE).

E.g. for the x64_86 and 48 bits in the phys addresses: the VAs have 9+9+9+9 bits per
each page table hierarchy + 12 bits for the page offset in 4KiB (1<<12) pages. That
also can be played as 9+9+9 and 21 bits which gives 3 level paging and 2MiB (1<<21)
pages but still _48_ bits for the VA and the phys. address. Sure can be 9+9 bits for
2-level pages and 30 bits for the page offset (1GiB pages, 1 << 30). Still, _48_
bits. Similar aruphmetic goes for _52_ bit addresses but going to need 5-level pages
this time.

Now, can you have 53 bits in the phys address (to address 1<<53 bytes)? No, you
cannot as that doesn't work with the existing x86_64 MMUs (and ARM64) - no virtual
address can be constructed to address 53 bits - not enough levels in the page table
hierarchy. As no VA can be constructed, the code won't be able to access the data.

Neither the EPT/2nd level set up by the hypervisor, nor for 1st level translation
set up by the guest or the host can go higher than 52 bits, even for MMIO. When MMIO
happens, that's a 2nd level fault/not present page and goes to the hypervisor but
_still_ the hv builds the EPT/2nd level map by the rules of the architecture and
cannot go above the architecturally imposed limits.

--
Cheers,
Roman

> 
> Thanks,
> Junrui Luo