[PATCH] Limit irq nesting

[PATCH] Limit irq nesting

[Mikulas Patocka]

Hi

During examination of stack-overflows on sparc64, it was found that there 
is no limit for a number of nested IRQ handlers. Sparc64 is especially 
stack-hungry architecture, minimum frame size is 192 bytes and after 75 
frames it overflows.

If someone provides a legitimate reason for more than 2 nested handlers, 
you could increase the constant in the patch --- but there really should 
be some limit, so that many simultaneous interrupts can't blow the stack.

Mikulas

---

IRQs without IRQF_DISABLED could nest to arbitrary level.

At worst this would mean having as many IRQ handlers stack frames, as there
are interrupts registered --- enough to cause a stack overflow.

This patch makes a limit to have at most two handlers on the stack.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
 include/linux/interrupt.h |    7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

Index: linux-2.6.26-rc8/include/linux/interrupt.h
===================================================================
--- linux-2.6.26-rc8.orig/include/linux/interrupt.h	2008-06-25 03:58:20.000000000 +0200
+++ linux-2.6.26-rc8/include/linux/interrupt.h	2008-07-01 17:42:44.000000000 +0200
@@ -16,6 +16,11 @@
 #include <asm/system.h>
 
 /*
+ * Max number of interrupt handlers on a stack. To prevent stack overflow.
+ */
+#define MAX_NESTED_INTERRUPTS	2
+
+/*
  * These correspond to the IORESOURCE_IRQ_* defines in
  * linux/ioport.h to select the interrupt line behaviour.  When
  * requesting an interrupt without specifying a IRQF_TRIGGER, the
@@ -95,7 +100,7 @@ extern void devm_free_irq(struct device 
 #ifdef CONFIG_LOCKDEP
 # define local_irq_enable_in_hardirq()	do { } while (0)
 #else
-# define local_irq_enable_in_hardirq()	local_irq_enable()
+# define local_irq_enable_in_hardirq()	do { if (hardirq_count() < (MAX_NESTED_INTERRUPTS << HARDIRQ_SHIFT)) local_irq_enable(); } while (0)
 #endif
 
 extern void disable_irq_nosync(unsigned int irq);

Re: [PATCH] Limit irq nesting

[KOSAKI Motohiro]

Hi Mikulas,

> ---
>  include/linux/interrupt.h |    7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> Index: linux-2.6.26-rc8/include/linux/interrupt.h
> ===================================================================
> --- linux-2.6.26-rc8.orig/include/linux/interrupt.h	2008-06-25 03:58:20.000000000 +0200
> +++ linux-2.6.26-rc8/include/linux/interrupt.h	2008-07-01 17:42:44.000000000 +0200
> @@ -16,6 +16,11 @@
>  #include <asm/system.h>
>  
>  /*
> + * Max number of interrupt handlers on a stack. To prevent stack overflow.
> + */
> +#define MAX_NESTED_INTERRUPTS	2
> +
> +/*
>   * These correspond to the IORESOURCE_IRQ_* defines in
>   * linux/ioport.h to select the interrupt line behaviour.  When
>   * requesting an interrupt without specifying a IRQF_TRIGGER, the

I have no objection to your approach.
but I don't know MAX_NESTED_INTERRUPTS is properly or not.

Could you explain why MAX_NESTED_INTERRUPTS is two?
Is it architecture independent properly number?



> @@ -95,7 +100,7 @@ extern void devm_free_irq(struct device 
>  #ifdef CONFIG_LOCKDEP
>  # define local_irq_enable_in_hardirq()	do { } while (0)
>  #else
> -# define local_irq_enable_in_hardirq()	local_irq_enable()
> +# define local_irq_enable_in_hardirq()	do { if (hardirq_count() < (MAX_NESTED_INTERRUPTS << HARDIRQ_SHIFT)) local_irq_enable(); } while (0)
>  #endif
>  
>  extern void disable_irq_nosync(unsigned int irq);

Re: [PATCH] Limit irq nesting

[Mikulas Patocka]

On Thu, 17 Jul 2008, KOSAKI Motohiro wrote:

> Hi Mikulas,
> 
> > ---
> >  include/linux/interrupt.h |    7 ++++++-
> >  1 file changed, 6 insertions(+), 1 deletion(-)
> > 
> > Index: linux-2.6.26-rc8/include/linux/interrupt.h
> > ===================================================================
> > --- linux-2.6.26-rc8.orig/include/linux/interrupt.h	2008-06-25 03:58:20.000000000 +0200
> > +++ linux-2.6.26-rc8/include/linux/interrupt.h	2008-07-01 17:42:44.000000000 +0200
> > @@ -16,6 +16,11 @@
> >  #include <asm/system.h>
> >  
> >  /*
> > + * Max number of interrupt handlers on a stack. To prevent stack overflow.
> > + */
> > +#define MAX_NESTED_INTERRUPTS	2
> > +
> > +/*
> >   * These correspond to the IORESOURCE_IRQ_* defines in
> >   * linux/ioport.h to select the interrupt line behaviour.  When
> >   * requesting an interrupt without specifying a IRQF_TRIGGER, the
> 
> I have no objection to your approach.
> but I don't know MAX_NESTED_INTERRUPTS is properly or not.
> 
> Could you explain why MAX_NESTED_INTERRUPTS is two?
> Is it architecture independent properly number?

I assume that there is at most one active bad-behaving device with long 
interrupt processing time. For example IDE disk in PIO mode.

If someone starts whining that he is simultaneously using two disks in PIO 
mode and is losing characters on serial line, we can make an architecture 
override for this (after we check that the architecture has enough stack 
space to hold more interrupts). But PIO mode is dead anyway, so there's no 
need to preemptively care about it.

Mikulas

[PATCH] limit irq nesting

[Mikulas Patocka]

Another potential problem (found during code review) that could cause 
stack overflow is indefinite irq nesting. Linux doesn't have any limit on 
number of nested irq handlers, so there may be as many handlers on a stack 
as there are registered hardware interrupts --- enough to cause a crash.

This patch limits interrupt nesting to at most 2 levels.

--

IRQs without IRQF_DISABLED could nest to arbitrary level.

At worst this would mean having as many IRQ handlers stack frames, as there
are interrupts registered --- enough to cause a stack overflow.

This patch makes a limit to have at most two handlers on the stack.

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>

Index: linux-2.6.26-rc7-devel/include/linux/interrupt.h
===================================================================
--- linux-2.6.26-rc7-devel.orig/include/linux/interrupt.h	2008-06-23 17:47:16.000000000 +0200
+++ linux-2.6.26-rc7-devel/include/linux/interrupt.h	2008-06-23 18:02:40.000000000 +0200
@@ -16,6 +16,11 @@
  #include <asm/system.h>

  /*
+ * Max number of interrupt handlers on a stack. To prevent stack overflow.
+ */
+#define MAX_NESTED_INTERRUPTS	2
+
+/*
   * These correspond to the IORESOURCE_IRQ_* defines in
   * linux/ioport.h to select the interrupt line behaviour.  When
   * requesting an interrupt without specifying a IRQF_TRIGGER, the
@@ -95,7 +100,7 @@
  #ifdef CONFIG_LOCKDEP
  # define local_irq_enable_in_hardirq()	do { } while (0)
  #else
-# define local_irq_enable_in_hardirq()	local_irq_enable()
+# define local_irq_enable_in_hardirq()	do { if (hardirq_count() < (MAX_NESTED_INTERRUPTS << HARDIRQ_SHIFT)) local_irq_enable(); } while (0)
  #endif

  extern void disable_irq_nosync(unsigned int irq);