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

Add a check in process_shm() to validate that the shared memory
guest address range does not overlap with any of the domain's
allocated RAM banks.

Signed-off-by: Joan Bae <joan.bae@boeing.com>
---
 xen/common/device-tree/static-shmem.c | 37 +++++++++++++++++++++++++++
 1 file changed, 37 insertions(+)

diff --git a/xen/common/device-tree/static-shmem.c b/xen/common/device-tree/static-shmem.c
index 4c4cc1b123..b0ae0304a1 100644
--- a/xen/common/device-tree/static-shmem.c
+++ b/xen/common/device-tree/static-shmem.c
@@ -293,6 +293,31 @@ static bool __init save_map_heap_pages(struct domain *d, struct page_info *pg,
     return false;
 }
 
+static bool __init
+check_shm_guest_paddr_overlap(struct kernel_info *kinfo, paddr_t gbase,
+                                paddr_t size)
+{
+    unsigned int i;
+    const struct membanks *kinfo_mem = kernel_info_get_mem_const(kinfo);
+    paddr_t gend = gbase + size;
+
+    for ( i = 0; i < kinfo_mem->nr_banks; i++ )
+    {
+        paddr_t bankbase = kinfo_mem->bank[i].start;
+        paddr_t bankend = bankbase + kinfo_mem->bank[i].size;
+
+        /* Check if shared memory overlaps with guest RAM */
+        if ( gbase < bankend && bankbase < gend )
+        {
+            printk("Shared memory guest address 0x%"PRIpaddr" - 0x%"PRIpaddr""
+                    " overlaps with guest RAM 0x%"PRIpaddr" - 0x%"PRIpaddr"\n",
+                    gbase, gend - 1, bankbase, bankend - 1);
+            return true;
+        }
+    }
+
+    return false;
+}
+
 int __init process_shm(struct domain *d, struct kernel_info *kinfo,
                        const struct dt_device_node *node)
 {
@@ -355,6 +380,12 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo,
             /* guest phys address is after host phys address */
             gbase = dt_read_paddr(cells + addr_cells, addr_cells);
 
+            if ( check_shm_guest_paddr_overlap(kinfo, gbase, psize) )
+            {
+                printk("%pd: shared memory region overlaps with the guest's RAM range\n", d);
+                return -EINVAL;
+            }
+
             if ( is_domain_direct_mapped(d) && (pbase != gbase) )
             {
                 printk("%pd: physical address 0x%"PRIpaddr" and guest address 0x%"PRIpaddr" are not direct-mapped.\n",
@@ -396,6 +427,12 @@ int __init process_shm(struct domain *d, struct kernel_info *kinfo,
             /* guest phys address is right at the beginning */
             gbase = dt_read_paddr(cells, addr_cells);
 
+            if ( check_shm_guest_paddr_overlap(kinfo, gbase, psize) )
+            {
+                printk("%pd: shared memory region overlaps with the guest's RAM range\n", d);
+                return -EINVAL;
+            }
+
             if ( !alloc_bank )
             {
                 alloc_heap_pages_cb_extra cb_arg = { d, role_str, gbase,
-- 
2.43.0

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,

-- 
Julien Grall

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

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