Re: [PATCH 2/3] domctl: also pause domain for extended context updates

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

[-- Attachment #1.1: Type: text/plain, Size: 2508 bytes --]

On 05/02/14 14:54, Jan Beulich wrote:
> This is not just for consistency with "base" context updates, but
> actually needed so that guest side accesses can't race with control
> domain side updates.
>
> This would have been a security issue if XSA-77 hadn't waived them on
> the affected domctl operation.
>
> While looking at the code I also spotted a redundant NULL check in the
> "base" context update handling code, which is being removed.
>
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

>
> --- a/xen/arch/x86/domctl.c
> +++ b/xen/arch/x86/domctl.c
> @@ -853,6 +853,8 @@ long arch_do_domctl(
>          }
>          else
>          {
> +            if ( d == current->domain ) /* no domain_pause() */
> +                break;
>              ret = -EINVAL;
>              if ( evc->size < offsetof(typeof(*evc), vmce) )
>                  break;
> @@ -861,6 +863,7 @@ long arch_do_domctl(
>                  if ( !is_canonical_address(evc->sysenter_callback_eip) ||
>                       !is_canonical_address(evc->syscall32_callback_eip) )
>                      break;
> +                domain_pause(d);
>                  fixup_guest_code_selector(d, evc->sysenter_callback_cs);
>                  v->arch.pv_vcpu.sysenter_callback_cs      =
>                      evc->sysenter_callback_cs;
> @@ -881,6 +884,8 @@ long arch_do_domctl(
>                        (evc->syscall32_callback_cs & ~3) ||
>                        evc->syscall32_callback_eip )
>                  break;
> +            else
> +                domain_pause(d);
>  
>              BUILD_BUG_ON(offsetof(struct xen_domctl_ext_vcpucontext,
>                                    mcg_cap) !=
> @@ -899,6 +904,8 @@ long arch_do_domctl(
>              }
>              else
>                  ret = 0;
> +
> +            domain_unpause(d);
>          }
>      }
>      break;
> --- a/xen/common/domctl.c
> +++ b/xen/common/domctl.c
> @@ -334,10 +334,6 @@ long do_domctl(XEN_GUEST_HANDLE_PARAM(xe
>          unsigned int vcpu = op->u.vcpucontext.vcpu;
>          struct vcpu *v;
>  
> -        ret = -ESRCH;
> -        if ( d == NULL )
> -            break;
> -
>          ret = -EINVAL;
>          if ( (d == current->domain) || /* no domain_pause() */
>               (vcpu >= d->max_vcpus) || ((v = d->vcpu[vcpu]) == NULL) )
>
>
>
>
>
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel


[-- Attachment #1.2: Type: text/html, Size: 3359 bytes --]

[-- Attachment #2: Type: text/plain, Size: 126 bytes --]

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