Re: [Xen-devel] [PATCH 2/5] x86/hvm: Switch hvm_allow_set_param() to use a whitelist

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Sep 07, 2018 at 07:13:08PM +0100, Andrew Cooper wrote:
> On 07/09/18 17:01, Roger Pau Monné wrote:
> > On Wed, Sep 05, 2018 at 07:12:01PM +0100, Andrew Cooper wrote:
> >> There are holes in the HVM_PARAM space, some of which are from deprecated
> >> parameters, but toolstack and device models currently has (almost) blanket
> >> write access.
> >>
> >> Rearrange hvm_allow_get_param() to have a whitelist of toolstack-writeable
> >> parameters, with the default case failing with -EINVAL.  This subsumes the
> >> HVM_NR_PARAMS check, as well as the MEMORY_EVENT_* deprecated block, and 
> >> the
> >> BUFIOREQ_EVTCHN Xen-write-only value.
> >>
> >> No expected change for the defined, in-use params.
> >>
> >> Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
> > Reviewed-by: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >
> >> ---
> >> CC: Jan Beulich <JBeulich@xxxxxxxx>
> >> CC: Wei Liu <wei.liu2@xxxxxxxxxx>
> >> CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
> >> CC: Paul Durrant <paul.durrant@xxxxxxxxxx>
> >> CC: Stefano Stabellini <sstabellini@xxxxxxxxxx>
> >> CC: Julien Grall <julien.grall@xxxxxxx>
> >> ---
> >>  xen/arch/x86/hvm/hvm.c | 53 
> >> +++++++++++++++++++++++++++++---------------------
> >>  1 file changed, 31 insertions(+), 22 deletions(-)
> >>
> >> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> >> index 96a6323..d19ae35 100644
> >> --- a/xen/arch/x86/hvm/hvm.c
> >> +++ b/xen/arch/x86/hvm/hvm.c
> >> @@ -4073,7 +4073,7 @@ static int hvm_allow_set_param(struct domain *d,
> >>  
> >>      switch ( a->index )
> >>      {
> >> -    /* The following parameters can be set by the guest. */
> >> +        /* The following parameters can be set by the guest and 
> >> toolstack. */
> >>      case HVM_PARAM_CALLBACK_IRQ:
> >>      case HVM_PARAM_VM86_TSS:
> > Note sure about the point of letting the guest set the unreal mode
> > TSS, but anyway this is not the scope of the patch.
> 
> Because hvmloader still sets it up for HVM guests.
> 
> Neither you nor Jan took my hints (when doing various related work) that
> unifying the PVH and HVM paths in the domain builder (alongside
> IDENT_PT) would be a GoodThing(tm).
> 
> OTOH, we do now actually have a fairly simple cleanup task which a
> student could be guided through doing, which would allow us to remove
> guest access to these two params.

Hm, right. The main problem I see with this is that the hypervisor has
no knowledge of the memory map when building a DomU (all this is in
the toolstack), so it's quite hard to figure out where to place the
TSS or the identity page tables.

We could make the special pages addresses somehow part of the public
headers, so that there's a fixed address known by both the toolstack
and the hypervisor about the location of those magic pages.

Roger.

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