x86/p2m: mapping a grant of a domain's own page on HVM/PVH unmaps the original gfn

[hippalectryon@xxxxxxxxxxxxxx]

Hello,

On x86 HVM/PVH, when a domain maps a grant that it issued to itself
(granting domid == own domid), the original gfn of the granted page
loses its backing. I ran into this while looking at why Qubes OS
blocks vchan connections from a domain to itself:
https://github.com/QubesOS/qubes-issues/issues/951

I tested this on Xen 4.19.5 (Qubes OS R4.3 build, running nested)
with a PVH Linux 6.18 guest, from inside the guest using libxengnttab
(gntalloc + gntdev) and libxenevtchn with the guest's own domid as the
peer. Here's what I see:

- xengntshr_share_pages() to own domid, then
  xengnttab_map_grant_ref() of that ref: both succeed.
- The new mapping reads the page's old contents (zeros). The
  granter's own mapping of the page then reads all 0xff, and writes
  through it are lost. This stays the case after unmap.
- Seconds later the guest oopses in fork (__split_huge_pmd_locked, a
  page table entry read as all ones): Linux reused the lost frame as a
  page table.
- With libxenvchan the server reads the ring's cli_live byte as 0xff
  and both sides hang.
- Event channels bound to self work fine. Without the map step the
  shared page behaves normally.

Reading the code, I believe the cause is in p2m_add_page(), which the
grant map reaches via create_grant_p2m_mapping(). Its second loop
looks up the M2P entry of every frame owned by d, and if the frame is
already mapped at another gfn, it treats the new mapping as a move and
calls p2m_remove_entry() on the old gfn. For a foreign grant owner != d
and the loop skips it, but for a grant of d's own page the original gfn
gets removed. On unmap, replace_grant_p2m_mapping() only removes the
grant entry, so the original gfn never comes back.

The check dates from b37d30c8fc ("Mapping grant references into HVM
guests, take 2"), whose comment said "If we get here, we know the
local domain owns the page, so it can't have been grant mapped in".

This change fixes it for me. I tested it on 4.19.5 with the Qubes
patches, and it applies to current staging:

--- a/xen/arch/x86/mm/p2m.c
+++ b/xen/arch/x86/mm/p2m.c
@@ p2m_add_page()
-        if ( owner != d )
+        /*
+         * A grant mapping of a domain's own page adds a second gfn for it;
+         * the page must stay mapped at its original gfn.
+         */
+        if ( owner != d || p2m_is_grant(t) )
             continue;

p2m_add_page() already leaves M2P alone for grant types, and unmap
goes through p2m_remove_entry(), which leaves M2P alone for
P2M_SPECIAL_TYPES, so the original gfn's M2P entry stays valid.

With this change, the self-mapped page reads and writes correctly in
both directions, the original mapping still works after unmap, there
is no oops, and libxenvchan loopback works. After also removing
qrexec's loopback checks, qrexec calls and file copies from a guest to
itself worked too, including concurrent calls and killing both ends
in the middle of a transfer. A build of the same tree without the
change fails the same way as stock.

I haven't tested PV guests (I couldn't boot one on this setup),
migration or log-dirty mode, or Xen versions other than 4.19.5.

I can't send a signed-off patch myself. If the fix looks right, could
someone pick it up, or suggest a better approach (for example
rejecting such mappings instead)?

Thanks,
Hippalectryon