Re: [Xen-devel] [PATCH v2] x86/HVM: restrict permitted instructions during special purpose emulation

[Paul Durrant <Paul.Durrant@xxxxxxxxxx>]

> -----Original Message-----
> From: Jan Beulich [mailto:JBeulich@xxxxxxxx]
> Sent: 05 January 2017 10:55
> To: xen-devel <xen-devel@xxxxxxxxxxxxxxxxxxxx>
> Cc: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>; Andrew
> Cooper <Andrew.Cooper3@xxxxxxxxxx>; Paul Durrant
> <Paul.Durrant@xxxxxxxxxx>; Jun Nakajima <jun.nakajima@xxxxxxxxx>; Kevin
> Tian <kevin.tian@xxxxxxxxx>; Boris Ostrovsky <boris.ostrovsky@xxxxxxxxxx>
> Subject: [PATCH v2] x86/HVM: restrict permitted instructions during special
> purpose emulation
> 
> Most invocations of the instruction emulator are for VM exits where the
> set of legitimate instructions (i.e. ones capable of causing the
> respective exit) is rather small. Restrict the permitted sets via a new
> callback, at once eliminating the abuse of handle_mmio() for non-MMIO
> operations.
> 
> A seemingly unrelated comment adjustment is being done here to keep
> x86_emulate() in sync with x86_insn_is_mem_write() (in the context of
> which this was found to be wrong).
> 
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
> ---
> v2: Add callback to hvm_ud_intercept() for the non-FEP case. Mention
>     comment adjustment. Eliminate hvm_emulate_is_*() wrappers around
>     x86_insn_is_*() calls, and convert a few from the former to the
>     latter.
> 
> Note that hvm_emulate_is_mem_*() (for now) intentionally don't include
> implicit memory operands: I don't think we mean to support namely
> the stack to live in MMIO, but otoh we may need to permit that.
> 
[snip]
> --- a/xen/include/asm-x86/hvm/emulate.h
> +++ b/xen/include/asm-x86/hvm/emulate.h
> @@ -17,9 +17,18 @@
>  #include <asm/hvm/hvm.h>
>  #include <asm/x86_emulate.h>
> 
> +typedef bool hvm_emulate_validate_t(const struct x86_emulate_state
> *state,
> +                                    const struct x86_emulate_ctxt *ctxt);
> +
>  struct hvm_emulate_ctxt {
>      struct x86_emulate_ctxt ctxt;
> 
> +    /*
> +     * validate: Post-decode, pre-emulate hook to allow caller controlled
> +     * filtering.
> +     */
> +    hvm_emulate_validate_t *validate;
> +
>      /* Cache of 16 bytes of instruction. */
>      uint8_t insn_buf[16];
>      unsigned long insn_buf_eip;
> @@ -41,6 +50,8 @@ enum emul_kind {
>      EMUL_KIND_SET_CONTEXT_INSN
>  };
> 
> +bool __nonnull(1) hvm_emulate_one_insn(
> +    hvm_emulate_validate_t *validate);
>  int hvm_emulate_one(
>      struct hvm_emulate_ctxt *hvmemul_ctxt);
>  void hvm_emulate_one_vm_event(enum emul_kind kind,
> @@ -49,6 +60,7 @@ void hvm_emulate_one_vm_event(enum emul_
>  /* Must be called once to set up hvmemul state. */
>  void hvm_emulate_init_once(
>      struct hvm_emulate_ctxt *hvmemul_ctxt,
> +    hvm_emulate_validate_t *validate,
>      struct cpu_user_regs *regs);
>  /* Must be called once before each instruction emulated. */
>  void hvm_emulate_init_per_insn(
> @@ -68,6 +80,11 @@ struct segment_register *hvmemul_get_seg
>      struct hvm_emulate_ctxt *hvmemul_ctxt);
>  int hvm_emulate_one_mmio(unsigned long mfn, unsigned long gla);
> 
> +static inline bool handle_mmio(void)
> +{
> +    return hvm_emulate_one_insn(x86_insn_is_mem_access);
> +}
> +

There are not many call sites for handle_mmio(). Would it not be better just to 
change them all rather than using this inline for the few left after your 
changes to the SVM code?

  Paul

>  int hvmemul_insn_fetch(enum x86_segment seg,
>                         unsigned long offset,
>                         void *p_data,
> --- a/xen/include/asm-x86/hvm/io.h
> +++ b/xen/include/asm-x86/hvm/io.h
> @@ -118,7 +118,6 @@ void relocate_portio_handler(
> 
>  void send_timeoffset_req(unsigned long timeoff);
>  void send_invalidate_req(void);
> -bool handle_mmio(void);
>  bool handle_mmio_with_translation(unsigned long gla, unsigned long gpfn,
>                                    struct npfec);
>  bool handle_pio(uint16_t port, unsigned int size, int dir);
> 


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
https://lists.xen.org/xen-devel