Re: [Xen-devel] [PATCH 4.1] x86: fix emuirq regression from XSA-21 fix

[DuanZhenzhong <zhenzhong.duan@xxxxxxxxxx>]

Jan Beulich wrote:
> > > > On 25.06.13 at 07:33, DuanZhenzhong <zhenzhong.duan@xxxxxxxxxx> wrote:
> > > >         
> > Stefano Stabellini wrote:
> >     
> > > On Mon, 24 Jun 2013, Zhenzhong Duan wrote:
> > >       
> > > > Could you have a look if there is something wrong in xen side of clearing 
> > > > the mapping?
> > > >         
> > > What I am saying is that the error you are getting:
> > >
> > > pt_msix_disable: Unbind msix with pirq 67, gvec 0
> > > pt_msix_disable: Unmap msix with pirq 67
> > > pt_msix_disable: Error: Unmapping of MSI-X failed. [00:04.0]
> > >
> > > cannot be caused by domain_pirq_to_emuirq(d, pirq) returning
> > > IRQ_UNBOUND.
> > > So, why are you getting this error? What is failing?
> > > I am ready to believe the problem is in Xen but Without understanding
> > > why you are getting the error it's hard to find a solution.
> > >   
> > >       
> > I found the reason, you are looking at xen-unstable, I was working with 
> > 4.1.30-OVM, it has patch of CVE-2012-4536 / XSA-21.
> > That patch set ret to -EINVAL initially. After remove that line, unmap 
> > succeed.
> >     
> Removing that line certainly isn't right. The proper fix is the one
> below/attached.
>
> Jan
>
> ****************************************************
> x86: fix emuirq regression from XSA-21 fix
>
> The XSA-21 patch broke the assumption of "ret" being zero prior to the
> IRQ_UNBOUND check.
>
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>
>
> --- a/xen/arch/x86/physdev.c
> +++ b/xen/arch/x86/physdev.c
> @@ -243,6 +243,8 @@ static int physdev_unmap_pirq(struct phy
>          spin_lock(&d->event_lock);
>          if ( domain_pirq_to_emuirq(d, unmap->pirq) != IRQ_UNBOUND )
>              ret = unmap_domain_pirq_emuirq(d, unmap->pirq);
> +        else
> +            ret = 0;
>          spin_unlock(&d->event_lock);
>          if ( unmap->domid == DOMID_SELF || ret )
>              goto free_domain;
>
>
>
>   
Any reason of not right? below patch does work as ret is already zero if it 
could get to that line.

zduan

****************************************************
x86: fix emuirq regression from XSA-21 fix

The XSA-21 patch broke the assumption of "ret" being zero prior to the
IRQ_UNBOUND check.

Signed-off-by: Zhenzhong Duan <zhenzhong.duan@xxxxxxxxxx>

--- a/xen/arch/x86/physdev.c
+++ b/xen/arch/x86/physdev.c
@@ -240,7 +240,6 @@ static int physdev_unmap_pirq(struct phy
    if ( ret )
        return ret;

-    ret = -EINVAL;
    if ( unmap->pirq < 0 || unmap->pirq >= d->nr_pirqs )
        goto free_domain;



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