Re: [PATCH v17 1/2] mem_sharing: fix sharability check during fork reset

[Tamas K Lengyel <tamas@xxxxxxxxxxxxx>]

On Fri, Apr 24, 2020 at 3:44 AM Roger Pau Monné <roger.pau@xxxxxxxxxx> wrote:
>
> On Thu, Apr 23, 2020 at 08:30:06AM -0700, Tamas K Lengyel wrote:
> > When resetting a VM fork we ought to only remove pages that were allocated 
> > for
> > the fork during it's execution and the contents copied over from the parent.
> > This can be determined if the page is sharable as special pages used by the
> > fork for other purposes will not pass this test. Unfortunately during the 
> > fork
> > reset loop we only partially check whether that's the case. A page's type 
> > may
> > indicate it is sharable (pass p2m_is_sharable) but that's not a sufficient
> > check by itself. All checks that are normally performed before a page is
> > converted to the sharable type need to be performed to avoid removing pages
> > from the p2m that may be used for other purposes. For example, currently the
> > reset loop also removes the vcpu info pages from the p2m, potentially 
> > putting
> > the guest into infinite page-fault loops.
> >
> > Signed-off-by: Tamas K Lengyel <tamas.lengyel@xxxxxxxxx>
>
> Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>

Thanks!

>
> I've been looking however and I'm not able to spot where you copy the
> shared_info data, which I think it's also quite critical for the
> domain, as it contains the info about event channels (when using L2).
> Also for HVM forks the shared info should be mapped at the same gfn as
> the parent, or else the child trying to access it will trigger an EPT
> fault that won't be able to populate such gfn, because the shared_info
> on the parent is already shared between Xen and the parent.

Pages that cause an EPT fault but can't be made shared get a new page
allocated for them and copied in mem_sharing_fork_page. There are many
pages like that, mostly due to QEMU mapping them and thus holding an
extra reference. That said, shared_info is manually copied during
forking in copy_special_pages, at the end of the function you will
find:

old_mfn = _mfn(virt_to_mfn(d->shared_info));
new_mfn = _mfn(virt_to_mfn(cd->shared_info));

copy_domain_page(new_mfn, old_mfn);

Cheers,
Tamas