[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> On 14.11.17 at 16:11, <apop@xxxxxxxxxxxxxxx> 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.

I fully agree with the change, but the description needs improvement.
For one, why would the domain be hanging with

static inline struct domain *rcu_lock_current_domain(void)
{
    return /*rcu_lock_domain*/(current->domain);
}

? And even if the lock function invocation wasn't commented
out, all it does is preempt_disable(). That may cause an
assertion to trigger in debug builds, but that's not a domain
hang. Plus ...

> --- 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;
> +        }

... you also change flow here, which is a second bug you address,
but you fail to mention it.

Jan


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

 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.