[PATCH] x86: fix an off-by-one pirq range check

[PATCH] x86: fix an off-by-one pirq range check

[Jan Beulich]

Signed-off-by: Jan Beulich <jbeulich@novell.com>

--- 2010-06-15.orig/xen/arch/x86/irq.c	2010-06-15 13:23:00.000000000 +0200
+++ 2010-06-15/xen/arch/x86/irq.c	2010-06-29 10:15:13.000000000 +0200
@@ -1027,7 +1027,7 @@ static void __pirq_guest_eoi(struct doma
 
 int pirq_guest_eoi(struct domain *d, int irq)
 {
-    if ( (irq < 0) || (irq > d->nr_pirqs) )
+    if ( (irq < 0) || (irq >= d->nr_pirqs) )
         return -EINVAL;
 
     __pirq_guest_eoi(d, irq);

Re: [PATCH] x86: fix an off-by-one pirq range check

[Konrad Rzeszutek Wilk]

On Wed, Jun 30, 2010 at 07:37:26AM +0100, Jan Beulich wrote:
> Signed-off-by: Jan Beulich <jbeulich@novell.com>

Won't that make PV guest with only one IRQ passed through unable to ACK it?
(and if the IRQ is not shared that is).

This code does get executed when the PHYSDEVOP_eoi hypercall is made I
believe.
> 
> --- 2010-06-15.orig/xen/arch/x86/irq.c	2010-06-15 13:23:00.000000000 +0200
> +++ 2010-06-15/xen/arch/x86/irq.c	2010-06-29 10:15:13.000000000 +0200
> @@ -1027,7 +1027,7 @@ static void __pirq_guest_eoi(struct doma
>  
>  int pirq_guest_eoi(struct domain *d, int irq)
>  {
> -    if ( (irq < 0) || (irq > d->nr_pirqs) )
> +    if ( (irq < 0) || (irq >= d->nr_pirqs) )
>          return -EINVAL;
>  
>      __pirq_guest_eoi(d, irq);
> 
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@lists.xensource.com
> http://lists.xensource.com/xen-devel

Re: [PATCH] x86: fix an off-by-one pirq range check

[Jan Beulich]

>>> On 30.06.10 at 18:16, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> On Wed, Jun 30, 2010 at 07:37:26AM +0100, Jan Beulich wrote:
>> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> 
> Won't that make PV guest with only one IRQ passed through unable to ACK it?
> (and if the IRQ is not shared that is).

Why would you think so? In such a case, nr_pirqs would be 1, and the
only permitted pirq would be 0. All other places do the range checks
correctly, just this one would let through an out of bounds number.

Jan

> This code does get executed when the PHYSDEVOP_eoi hypercall is made I
> believe.
>> 
>> --- 2010-06-15.orig/xen/arch/x86/irq.c	2010-06-15 13:23:00.000000000 +0200
>> +++ 2010-06-15/xen/arch/x86/irq.c	2010-06-29 10:15:13.000000000 +0200
>> @@ -1027,7 +1027,7 @@ static void __pirq_guest_eoi(struct doma
>>  
>>  int pirq_guest_eoi(struct domain *d, int irq)
>>  {
>> -    if ( (irq < 0) || (irq > d->nr_pirqs) )
>> +    if ( (irq < 0) || (irq >= d->nr_pirqs) )
>>          return -EINVAL;
>>  
>>      __pirq_guest_eoi(d, irq);
>> 
>> 
>> 
>> 
>> _______________________________________________
>> Xen-devel mailing list
>> Xen-devel@lists.xensource.com 
>> http://lists.xensource.com/xen-devel 

Re: [PATCH] x86: fix an off-by-one pirq range check

[Konrad Rzeszutek Wilk]

On Thu, Jul 01, 2010 at 08:27:23AM +0100, Jan Beulich wrote:
> >>> On 30.06.10 at 18:16, Konrad Rzeszutek Wilk <konrad.wilk@oracle.com> wrote:
> > On Wed, Jun 30, 2010 at 07:37:26AM +0100, Jan Beulich wrote:
> >> Signed-off-by: Jan Beulich <jbeulich@novell.com>
> > 
> > Won't that make PV guest with only one IRQ passed through unable to ACK it?
> > (and if the IRQ is not shared that is).
> 
> Why would you think so? In such a case, nr_pirqs would be 1, and the
> only permitted pirq would be 0. All other places do the range checks
> correctly, just this one would let through an out of bounds number.

Excellent. Thanks for the answer.