Re: [Xen-devel] [PATCH] x86/hvm: Fix rcu_unlock_domain call bypass

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 14/11/17 15:11, Adrian Pop wrote:
> rcu_lock_current_domain is called at the beginning of do_altp2m_op, but
> the altp2m_vcpu_enable_notify subop handler might skip calling
> rcu_unlock_domain, possibly hanging the domain altogether.
>
> Signed-off-by: Adrian Pop <apop@xxxxxxxxxxxxxxx>

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

CC'ing Julien.  This is 4.10 material IMO; it would be a security issue
if rcu_lock_current_domain() wasn't a nop in Xen.  Debug builds are also
liable to hit an assertion pertaining to the preempt_count() (which
again, is only ever read in debug builds).

> ---
>  xen/arch/x86/hvm/hvm.c | 8 +++++++-
>  1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/xen/arch/x86/hvm/hvm.c b/xen/arch/x86/hvm/hvm.c
> index 205b4cb685..0af498a312 100644
> --- a/xen/arch/x86/hvm/hvm.c
> +++ b/xen/arch/x86/hvm/hvm.c
> @@ -4534,12 +4534,18 @@ static int do_altp2m_op(
>  
>          if ( a.u.enable_notify.pad || a.domain != DOMID_SELF ||
>               a.u.enable_notify.vcpu_id != curr->vcpu_id )
> +        {
>              rc = -EINVAL;
> +            break;
> +        }
>  
>          if ( !gfn_eq(vcpu_altp2m(curr).veinfo_gfn, INVALID_GFN) ||
>               mfn_eq(get_gfn_query_unlocked(curr->domain,
>                      a.u.enable_notify.gfn, &p2mt), INVALID_MFN) )
> -            return -EINVAL;
> +        {
> +            rc = -EINVAL;
> +            break;
> +        }
>  
>          vcpu_altp2m(curr).veinfo_gfn = _gfn(a.u.enable_notify.gfn);
>          altp2m_vcpu_update_vmfunc_ve(curr);


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