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

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

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