Re: [PATCH v15 01/11] multicall: add no preemption ability between two calls

[Andrew Cooper <andrew.cooper3@citrix.com>]

On 05/09/14 09:37, Chao Peng wrote:
> Add a flag to indicate if the execution can be preempted between two
> calls. If not specified, stay preemptable.
>
> Signed-off-by: Chao Peng <chao.p.peng@linux.intel.com>
> ---
>  xen/common/multicall.c   |    5 ++++-
>  xen/include/public/xen.h |    4 ++++
>  2 files changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/xen/common/multicall.c b/xen/common/multicall.c
> index fa9d910..83b96eb 100644
> --- a/xen/common/multicall.c
> +++ b/xen/common/multicall.c
> @@ -40,6 +40,7 @@ do_multicall(
>      struct mc_state *mcs = &current->mc_state;
>      uint32_t         i;
>      int              rc = 0;
> +    bool_t           preemptable = 0;
>  
>      if ( unlikely(__test_and_set_bit(_MCSF_in_multicall, &mcs->flags)) )
>      {
> @@ -52,7 +53,7 @@ do_multicall(
>  
>      for ( i = 0; !rc && i < nr_calls; i++ )
>      {
> -        if ( i && hypercall_preempt_check() )
> +        if ( preemptable && hypercall_preempt_check() )
>              goto preempted;
>  
>          if ( unlikely(__copy_from_guest(&mcs->call, call_list, 1)) )
> @@ -61,6 +62,8 @@ do_multicall(
>              break;
>          }
>  
> +        preemptable = mcs->call.flags & MC_NO_PREEMPT;
> +

Please consider what would happen if a malicious guest set NO_PREEMPT on
every multicall entry.

IMO, two back-to-back entries whith NO_PREEMPT set is grounds for an
immediate hypercall failure.

Furthermore, what happens if one of the multicall operations pre-empts
of its own accord?

~Andrew

>          trace_multicall_call(&mcs->call);
>  
>          do_multicall_call(&mcs->call);
> diff --git a/xen/include/public/xen.h b/xen/include/public/xen.h
> index a6a2092..3f8b908 100644
> --- a/xen/include/public/xen.h
> +++ b/xen/include/public/xen.h
> @@ -550,10 +550,14 @@ DEFINE_XEN_GUEST_HANDLE(mmu_update_t);
>  struct multicall_entry {
>      xen_ulong_t op, result;
>      xen_ulong_t args[6];
> +    uint32_t flags;
>  };
>  typedef struct multicall_entry multicall_entry_t;
>  DEFINE_XEN_GUEST_HANDLE(multicall_entry_t);
>  
> +/* These flags passed in the 'flags' field of multicall_entry_t. */
> +#define MC_NO_PREEMPT    (1<<0)  /* Can't be preempted before next entry? */
> +
>  #if __XEN_INTERFACE_VERSION__ < 0x00040400
>  /*
>   * Event channel endpoints per domain (when using the 2-level ABI):