RE: [EXTERNAL] Re: [XEN PATCH] xen/common: validate shared memory guest address overlap with guest RAM

["Bae, Joan" <joan.bae@xxxxxxxxxx>]

Hi Julien,

> Hi Julien,
> 
> Thank you for the review. I agree that the overlap issue is not limited to 
> shared
> memory overlapping with RAM. It could happen with any P2M mapping
> during domain construction.
> 
> I researched the callers of p2m_set_entry(). At a higher level,
> p2m_insert_mapping() callers can be categorized into two groups: runtime
> hypercalls and domain construction.
> 
> Runtime hypercalls such as XENMEM_populate_physmap rely on overwriting
> existing mappings, so they must allow it. On the other hand, domain
> construction callers such as guest_physmap_add_pages() should not allow
> overwriting existing mappings.
> 
> Since both categories depend on p2m_set_entry(), adding a blanket check
> there would break the runtime hypercall paths.
> 
> My plan for v2 is to add a checked variant of p2m_insert_mapping() (named
> as p2m_insert_mapping_checked) that verifies no existing mapping is present
> before inserting. Domain build paths would use the checked version, while
> runtime hypercall paths remain unchanged.
> 
> I also noticed a related TODO in p2m.h:
> /* TODO: Add a check in __p2m_set_entry() to avoid creating a mapping in
> * arch_domain_create() that requires p2m_put_l3_page() to be called. /
> 
> This seems to be addressing a similar concern. Would the approach of a
> checked wrapper at the p2m_insert_mapping() level be acceptable, or would
> you prefer the check at a different level?
> 
> Thank you,
> Joan
> 
>> EXT email: be mindful of links/attachments.
>> 
>> Hi Joan,
>> 
>> Thank you for the patch.
>> 
>> On 14/04/2026 09:59, Joan Bae wrote:
>>> Currently, process_shm() does not check whether the guest physical
>>> address of a shared memory region overlaps with the domain's
>>> allocated RAM banks. Neither process_shm() nor p2m_set_entry() checks
>>> for existing mappings, so the RAM mapping is silently overwritten if
>>> a user specifies a guest physical address that falls within the guest
>>> RAM range. Since construct_domain() loads the kernel after
>>> process_shm(), the kernel can end up in shared memory pages. This can
>>> cause: - Another domain corrupting the kernel via shared memory write
>>> - Silent guest crash with no error message from Xen
>> 
>> This seems to be solving one specific issue (RAM clashing with shared
>> memory) but I believe this could also happen with other kind of
>> mappings because, as you said, p2m_set_entry() doesn't check any overlap.
>> 
>> So I would rather prefer if we solve the problem once and for all.
>> This would mean modifying p2m_set_entry() (or one of its top caller).
>> Although, we would need to be careful to not break memory hypercalls
>> which may rely on overwriting existing mappings.
>> 
>> Cheers,
>> 
> 
> Thanks,
> Joan Bae
>

Gentle ping on this one. Please let me know if the proposed direction looks 
right.

Thanks,
Joan Bae