Jan Beulich
2010-Jun-30 06:37 UTC
[Xen-devel] [PATCH] x86: fix an off-by-one pirq range check
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); _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2010-Jun-30 16:16 UTC
Re: [Xen-devel] [PATCH] x86: fix an off-by-one pirq range check
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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Jan Beulich
2010-Jul-01 07:27 UTC
Re: [Xen-devel] [PATCH] x86: fix an off-by-one pirq range check
>>> 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_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Konrad Rzeszutek Wilk
2010-Jul-01 16:19 UTC
Re: [Xen-devel] [PATCH] x86: fix an off-by-one pirq range check
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. _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel