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 03:05 PM, Jan Beulich wrote:
>>>> On 11.12.17 at 15:51, <george.dunlap@xxxxxxxxxx> wrote:
>> On 12/11/2017 02:46 PM, Razvan Cojocaru wrote:
>>> On 12/11/2017 03: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).
>>>
>>> Quite likely I'm not grasping the full meaning of your objection,
>>> however the added code is merely another interface to already existing
>>> core code - so while admittedly there's room for improvement for the EPT
>>> code below it, this patch really only extends the scope of altp2m's
>>> existing version of set_mem_access() (which currently works on a single
>>> page). In that, it at least doesn't seem to make things worse (it's
>>> really just an optimization - whatever badness this code can cause with
>>> a single call, can already be achieved exactly with a sequence of
>>> xc_altp2m_set_mem_access() calls).
>>
>> I think Jan was saying that he would ideally like to remove *all* guest
>> access to altp2m functionality, even what's currently there.  The more
>> extra features we make available to guests, the harder it will be in the
>> future to argue to remove it all.
> 
> With one slight correction: all _uncontrolled_ access is what I'd like
> to see removed. Right now this could arguably indeed mean all
> access, as it is all uncontrolled (afaict).

Well at the moment all guest altp2m functionality is disabled unless the
toolstack has set the appropriate HVM param.  Is that not sufficient?

 -George

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