Re: [Xen-devel] [PATCH v8] x86/altp2m: support for setting restrictions for an array of pages

[George Dunlap <george.dunlap@xxxxxxxxxx>]

On 12/11/2017 01:36 PM, Jan Beulich wrote:
>>>> On 11.12.17 at 13:50, <george.dunlap@xxxxxxxxxx> wrote:
>> On 12/11/2017 12:12 PM, Jan Beulich wrote:
>>>>>> On 11.12.17 at 12:06, <andrew.cooper3@xxxxxxxxxx> wrote:
>>>> My suggestion was that we don't break usecases.  The Intel usecase
>>>> specifically is for an in-guest entity to have full control of all
>>>> altp2m functionality, and this is fine (security wise) when permitted to
>>>> do so by the toolstack.
>>>
>>> IOW you mean that such guests would be considered "trusted", i.e.
>>> whatever bad they can do is by definition not a security concern.
>>
>> I'm not sure what you mean by "trusted".  If implemented correctly,
>> altp2m and mem_access shouldn't give the guest any more permissions than
>> it has already.  The main risk would be if there were bugs in the
>> functionality that allowed security issues.
> 
> Hmm, maybe I'm mis-reading the code, but
> mem_access.c:set_mem_access() looks to be using the requested
> access rights verbatim, i.e. without applying tool stack imposed
> restrictions (hypervisor ones look to be honored by deriving
> base permissions from the p2m type first).

Right, so one thing that would need to be fixed is to enumerate which
things should work with what.  At the moment it looks like toolstack
mem_access functionality is simply incompatible with altp2m mem_access
functionality.  If you want to use toolstack mem_access, don't enable
guest altp2m mem_access.

If they were both needed to run at the same time, we could 1) prevent
the guest from modifying the "host p2m" (altp2m index 0) and 2) make
sure the altp2m entries were at least as strict as the host p2m entries.

>> You argued that we should keep PV linear pagetables, before knowing that
>> NetBSD used them, in spite of having discovered two *actual*
>> vulnerabilities in the implementation.  I don't really see how this is
>> different.
> 
> It's quite the opposite to me - I don't see the similarity. On this
> thread we're talking about new functionality, and how far to
> expose it. PV linear page tables had been there (and considered
> supported) for years, so removing the functionality or even only
> calling it unsupported all of the sudden didn't seem right at all.

Well the idea of calling it unsupported was assuming that there weren't
many people using it; finding out that NetWare, and in particular
NetBSD, still used it changes the situation quite a bit.

What I remember you actually saying at the time was, "We have
functionality already, I don't see why we don't make it secure rather
than removing it."  The same kind of argument would seem to apply here:
We have functionality that allows a guest agent to manipulate its altp2m
access rights; why we don't make it secure rather than removing it?

I certainly agree we shouldn't call it security supported until it's had
a thorough audit; and I wouldn't do that work unless there was someone
who actually wanted that support.  But leaving the code in a state that
could be given security support whenever someone wants to pick it up
makes some sense (as long as it doesn't open up new attacks for guests
not using that feature).

 -George

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel