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

[Wei Liu <wei.liu2@xxxxxxxxxx>]

On Tue, Jul 10, 2018 at 10:33:22AM +0100, George Dunlap wrote:
> The altp2m functionality was originally envisioned to be used in
> several different configurations, one of which was a single in-guest
> agent that had full operational control of altp2m.  This required the
> single hypercall to be an HVMOP, which is the only type of hypercall
> an HVM guest is allowed to make.
> 
> Exposing the altp2m functionality to the guest was controversial at
> the time, but was ultimately accepted.  The fact that altp2m is an
> HVMOP rather than a DOMCTL has caused some problems, however, for
> those moving forward trying to extend the interface: Extending the
> interface even for the 'external' use case now means extending an
> HVMOP, which implicitly extends the surface of attack for the
> 'internal' use case as well.  The result has been that every addition
> to this interface has also been controversial.
> 
> Settle the controversy once and for all by documenting 1) the purpose
> of the altp2m interface, and 2) how to extend it.  In particular:
> 
> * Specify that the fully in-guest agent is a target use case
> 
> * Specify that all extensions to altp2m functionality should be subops
>   of the HVMOP hypercall
> 
> * Specify that new subops should be disabled in ALTP2M_mixed mode by
>   default, unless specifically evaluated as being useful for the
>   'internal' use case.
> 
> Hopefully this will allow the altp2m interface to be developed further
> without unnecessary controversy.
> 
> Signed-off-by: George Dunlap <george.dunlap@xxxxxxxxxx>
> ---
> CC: Ian Jackson <ian.jackson@xxxxxxxxxx>
> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> CC: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> CC: Jan Beulich <jbeulich@xxxxxxxx>
> CC: Tim Deegan <tim@xxxxxxx>
> CC: Konrad Wilk <konrad.wilk@xxxxxxxxxx>
> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> CC: Julien Grall <julien.grall@xxxxxxx>
> CC: Tamas K Lengyel <tamas.lengyel@xxxxxxxxxxxx>
> CC: Lars Kurth <lars.kurth@xxxxxxxxxx>
> 
> 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.

The second "internal" should be "external".

> 
> 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.
> 
> It just makes more sense to have all the altp2m operations in a single
> place, and a simple way to control whether they're available to the
> 'internal' use case or not.  As such, I am proposing 'C'.  I know Jan
> considers this "badness", and objects to the continual "extension" of
> the "badness", but I disagree, and I strongly object to the other two
> options.
> 
> Disabling new subops for the 'internal' use case by default means that
> we can add new subops without worrying about making the 'internal' use
> case less secure; but if in the future someone makes the case that
> they are safe and necessary, we can enable them without having code
> duplication.
> 
> In any case need to come to an agreement once and for all so that
> Tamas and Razvan can do their work without continual arguments over a
> mode they're not using.

Yes this is important.

> ---
>  xen/arch/x86/hvm/hvm.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
> 
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index e022f5ab0e..90a4be5e86 100644
> --- 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.

I don't understand this argument. There is no risk of code duplication /
fragmentation if the implementation is contained within a function.
Should we choose to split one HVMOP into a DOMCTL and a HVMOP, there is
now two entries to the internal function, each of which with proper
checks, but they will call the same internal function eventually.

I admit I haven't followed the discussion closely.

Wei.

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