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

["Jan Beulich" <JBeulich@xxxxxxxx>]

>>> 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).

Jan


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