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

[DuanZhenzhong <zhenzhong.duan@oracle.com>]

Jan Beulich wrote:
>>>> On 25.06.13 at 07:33, DuanZhenzhong <zhenzhong.duan@oracle.com> 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@suse.com>
>
> --- 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@oracle.com>

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