Re: [PATCH] libxc: introduce a per architecture scratch pfn for temporary grant mapping

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 13/01/15 20:10, Julien Grall wrote:
> The code to initialize the grant table in libxc uses
> xc_domain_maximum_gpfn() + 1 to get a guest pfn for mapping the grant
> frame and to initialize it.
>
> This solution has two major issues:
>     - The check of the return of xc_domain_maximum_gpfn is buggy because
>     xen_pfn_t is unsigned and in case of an error -ERRNO is returned.
>     Which is never catch with ( pfn <= 0 ).

Wow - xc_domain_maximum_gpfn() will currently truncate long to int on
64bit systems.  That is unhelpful of it.

>     - The guest memory layout maybe filled up to the end, i.e
>     xc_domain_maximum_gpfn() + 1 gives either 0 or an invalid PFN due to
>     hardware limitation.

I realise I am throwing a spanner in the works here, but if you are
looking to fix it, lets fix this properly rather than hacking around the
problem further.

There is currently no way for the toolstack to map something on behalf
of a guest which is not in the guest physmap.  As a workaround, the code
here shoots a guest ram page, adds a non-ram page to the physmap, maps,
edits, unmaps and replaces the ram.

This is inefficient, liable to shatter superpages, and likely to end up
with with the returned ram allocated from the wrong numa node.

Furthermore, we have had security vulnerabilities in the past because
toolstack/device model components use guest pages (because of no
alternate mechanism) for emulation state/ring buffers without preventing
the guest itself from having access if it can find them.

Both of these issues are caused by the underlying inability for the
toolstack to map anything other than gfn space.

In the general case, an "alloc/map emulation page for" interface would
fix the security issue side of things (and make some existing code far
more simple).

However, in this case a mapping hypercall for pages which a guest could
add to its own physmap (but are not necessarily present at the moment)
would be the correct solution.

~Andrew