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

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 15/01/15 10:45, Tim Deegan wrote:
> At 12:58 +0000 on 14 Jan (1421236725), Andrew Cooper wrote:
>> 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).
> Sounds like a good idea.  Adding a new per-guest address space of
> memory that is accessable to tools & xen but not to the guest, right?

Yes, although this email contains two orthogonal suggestions.

>
> Presumably the existing magic pages break down into those that the
> guest can see/DMA/&c, and those that live in this other address
> space.  AFAICS grant tables will be in the first category, though.

Correct.

For things like grant tables, the toolstack is already capable of using
add_to_physmap to make the pages mappable, but this is inefficient and
possibly interferes with the guest physical layout.  I propose a short
circuit of this which allows the toolstack to map any legitimate physmap
spaces directly, without having to shuffle them in and out of the
physmap. i.e. a map foreign hypercall which takes {domid, space, idx} as
parameters rather than {domid, gfn}.

For the magic pages, this proposal creates a secondary address space,
which is intended never for the guest to be able to map.  This can
remove all the current "magic pages" which live in the low MMIO hole
(ioreq, bufioreq, mem_event rings, etc), and prevents the need for
emulation pages ever to be accessible to the guest.

~Andrew


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel