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 16:54, <george.dunlap@xxxxxxxxxx> wrote:
> 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?

Together with someone enabling it rendering the system unsupported,
I can live with that (as said elsewhere). I'm probably just not going to
ack any patch similar to the one here (but I won't object to someone
else doing so).

Jan


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