[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

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


Xen-devel mailing list



Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.