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

Re: [Xen-devel] [PATCH] hvm/altp2m: Clarify the proper way to extend the altp2m interface



>>> On 10.07.18 at 12:30, <george.dunlap@xxxxxxxxxx> wrote:
> On 07/10/2018 11:01 AM, Jan Beulich wrote:
>>>>> On 10.07.18 at 11:33, <george.dunlap@xxxxxxxxxx> wrote:
>>> As far as I can tell there are three possible solutions to this
>>> controversy:
>>>
>>> A. Remove the 'internal' functionality as a target by converting the
>>> current HVMOP into a DOMCTL.
>>>
>>> B. Have two hypercalls -- an HVMOP which contains functionality
>>> expected to be used by the 'internal' agent, and a DOMCTL for
>>> functionality which is expected to be used only be the 'internal'
>>> agent.
>>>
>>> C. Agree to add all new subops to the current hypercall (HVMOP), even
>>> if we're not sure if they should be exposed to the guest.
>>>
>>> I think A is a terrible idea.  Having a single in-guest agent is a
>>> reasonable design choice, and apparently it was even implemented at
>>> some point; we should make it straightforward for someone in the
>>> future to pick up the work if they want to.
>>>
>>> I think B is also a terrible idea.  The people extending it at the
>>> moment are primarily concerned with the 'external' use case.  There is
>>> nobody around to represent whether new functionality should end up in
>>> the HVMOP or the DOMCTL, which means that by default it will end up in
>>> the DOMCTL.  If it is discovered, afterwards, that the new operations
>>> *would* be safe and useful for the 'internal' use case, then we will
>>> have to duplicate them inside the HVMOP.
>> 
>> They'd have to be _moved_ to HVMOP, not duplicated.
> 
> I'd assumed we would want to be backwards compatible with existing dom0
> agents.

No domctl should ever be considered stable, and no dom0 agent
should call them without going through the libxc wrapper.

>>> --- a/xen/arch/x86/hvm/hvm.c
>>> +++ b/xen/arch/x86/hvm/hvm.c
>>> @@ -4460,6 +4460,34 @@ static int hvmop_get_param(
>>>      return rc;
>>>  }
>>>  
>>> +/*
>>> + * altp2m operations are envisioned as being used in several different 
>>> + * modes:
>>> + * 
>>> + * - external: All control and decisions are made by an external agent
>>> + *   running domain 0.
>>> + *
>>> + * - internal: altp2m operations are used exclusively by an in-guest agent
>>> + *   to protect itself from the guest kernel and in-guest attackers.  
>>> + * 
>>> + * - coordinated: An in-guest agent handles #VE and VMFUNCs locally,
>>> + *   but makes requests of an external entity for bigger changes (such
>>> + *   as modifying altp2m entires).
>>> + *
>>> + * This corresponds to the three values for HVM_PARAM_ALTP2M
>>> + * (external, mixed, limited). All three models have advantages and
>>> + * disadvantages.
>>> + *
>>> + * Normally hypercalls made by a program in domain 0 in order to
>>> + * control a guest would be DOMCTLs rather than HVMOPs.  But in order
>>> + * to properly enable the 'internal' use case, as well as to avoid
>>> + * fragmentation, all altp2m subops should come under this single
>>> + * HVMOP.
>>> + *
>>> + * New subops which may not be suitable for the 'internal' use case
>>> + * should be disabled in the "XEN_ALTP2M_mixed" case in
>>> + * xsm_hvm_altp2mhvm_op().
>>> + */
>> 
>> To me this last statement sort of implies (due to the lack of any black
>> or white listing in xsm_hvm_altp2mhvm_op()'s XEN_ALTP2M_mixed
>> handling) that all current ops are considered safe for internal use,
>> which I highly doubt.
> Given a blacklist (or an invitation to make one), someone might indeed
> infer that the items not blacklisted had been evaluated and deemed safe
> to use.  We have two possible solutions:
> 
> 1. Go through and make such an evaluation, blacklisting everything we
> don't think is necessary / safe
> 
> 2. Write a comment saying that the interface hasn't been evaluated.
> 
> Or 2a: Include a comment saying the 'internal' interface hasn't been
> evaluated for safety, and don't bother blacklisting new ops until such
> an evaluation has been made.

I see no value in a black list if only future things would be added to it.
I'm okay with 2a, but I think 1 would make easier the future job of
auditing the whole lot for supportability.

Jan


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

 


Rackspace

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