Re: [Xen-devel] map_domain_emuirq_pirq() imbalance with unmap_domain_pirq_emuirq()?

[Stefano Stabellini <stefano.stabellini@xxxxxxxxxxxxx>]

On Wed, 14 Dec 2011, Jan Beulich wrote:
> >>> On 13.12.11 at 18:16, Stefano Stabellini 
> >>> <stefano.stabellini@xxxxxxxxxxxxx> wrote:
> > --- a/xen/arch/x86/physdev.c
> > +++ b/xen/arch/x86/physdev.c
> > @@ -216,14 +216,16 @@ int physdev_unmap_pirq(domid_t domid, int pirq)
> >      if ( ret )
> >          return ret;
> >  
> > -    if ( is_hvm_domain(d) )
> > +    if ( is_hvm_domain(d) && domain_pirq_to_emuirq(d, pirq) != IRQ_UNBOUND)
> >      {
> >          spin_lock(&d->event_lock);
> >          ret = unmap_domain_pirq_emuirq(d, pirq);
> >          spin_unlock(&d->event_lock);
> > -        if ( domid == DOMID_SELF || ret )
> > +        if ( ret )
> >              goto free_domain;
> >      }
> > +    if ( domid == DOMID_SELF )
> > +        goto free_domain;
> >  
> >      ret = -EPERM;
> >      if ( !IS_PRIV_FOR(current->domain, d) )
> 
> I think this is the correct change (untested so far):
> 
> @@ -228,7 +228,8 @@ static int physdev_unmap_pirq(struct phy
>      if ( is_hvm_domain(d) )
>      {
>          spin_lock(&d->event_lock);
> -        ret = unmap_domain_pirq_emuirq(d, pirq);
> +        if ( domain_pirq_to_emuirq(d, pirq) != IRQ_UNBOUND )
> +            ret = unmap_domain_pirq_emuirq(d, pirq);
>          spin_unlock(&d->event_lock);
>          if ( unmap->domid == DOMID_SELF || ret )
>              goto free_domain;
> 
> i.e. do the lookup with the lock held (and taking advantage of 'ret'
> being zero when reaching the enclosing if()).
 
ack

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel