LKML Archive on lore.kernel.org
help / color / mirror / Atom feed
* [PATCH] NULL struct irq_desc's member 'name' in dynamic_irq_cleanup()
@ 2008-10-16 12:58 Dean Nelson
  2008-10-16 23:11 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Dean Nelson @ 2008-10-16 12:58 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Ingo Molnar, linux-kernel

If the member 'name' of the irq_desc structure happens to point to a character
string that is resident within a kernel module, problems insue if that module
is rmmod'd (at which time dynamic_irq_cleanup() is called) and then later
show_interrupts() is called by someone. It is also not a good thing if the
character string resided in kmalloc'd space that has been kfree'd (after
having called dynamic_irq_cleanup()). dynamic_irq_cleanup() fails to NULL
the 'name' member and show_interrupts() references it on a few architectures
(like h8300, sh and x86).

Signed-off-by: Dean Nelson <dcn@sgi.com>

---

 kernel/irq/chip.c |    1 +
 1 file changed, 1 insertion(+)

Index: linux/kernel/irq/chip.c
===================================================================
--- linux.orig/kernel/irq/chip.c	2008-10-15 07:44:31.000000000 -0500
+++ linux/kernel/irq/chip.c	2008-10-16 06:55:56.000000000 -0500
@@ -79,6 +79,7 @@ void dynamic_irq_cleanup(unsigned int ir
 	desc->chip_data = NULL;
 	desc->handle_irq = handle_bad_irq;
 	desc->chip = &no_irq_chip;
+	desc->name = NULL;
 	spin_unlock_irqrestore(&desc->lock, flags);
 }
 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] NULL struct irq_desc's member 'name' in dynamic_irq_cleanup()
  2008-10-16 12:58 [PATCH] NULL struct irq_desc's member 'name' in dynamic_irq_cleanup() Dean Nelson
@ 2008-10-16 23:11 ` Andrew Morton
  2008-10-17 18:30   ` Dean Nelson
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2008-10-16 23:11 UTC (permalink / raw)
  To: Dean Nelson; +Cc: mingo, linux-kernel, stable

On Thu, 16 Oct 2008 07:58:08 -0500
Dean Nelson <dcn@sgi.com> wrote:

> If the member 'name' of the irq_desc structure happens to point to a character
> string that is resident within a kernel module, problems insue if that module
> is rmmod'd (at which time dynamic_irq_cleanup() is called) and then later
> show_interrupts() is called by someone.

It would be nice to spell out what the "problems" are.

> It is also not a good thing if the
> character string resided in kmalloc'd space that has been kfree'd (after
> having called dynamic_irq_cleanup()). dynamic_irq_cleanup() fails to NULL
> the 'name' member and show_interrupts() references it on a few architectures
> (like h8300, sh and x86).
> 
> Signed-off-by: Dean Nelson <dcn@sgi.com>
> 
> ---
> 
>  kernel/irq/chip.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> Index: linux/kernel/irq/chip.c
> ===================================================================
> --- linux.orig/kernel/irq/chip.c	2008-10-15 07:44:31.000000000 -0500
> +++ linux/kernel/irq/chip.c	2008-10-16 06:55:56.000000000 -0500
> @@ -79,6 +79,7 @@ void dynamic_irq_cleanup(unsigned int ir
>  	desc->chip_data = NULL;
>  	desc->handle_irq = handle_bad_irq;
>  	desc->chip = &no_irq_chip;
> +	desc->name = NULL;
>  	spin_unlock_irqrestore(&desc->lock, flags);
>  }
>  

Because we should work out whether this should be backported into
-stable.  And if so, how far back it should go.


^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] NULL struct irq_desc's member 'name' in dynamic_irq_cleanup()
  2008-10-16 23:11 ` Andrew Morton
@ 2008-10-17 18:30   ` Dean Nelson
  0 siblings, 0 replies; 3+ messages in thread
From: Dean Nelson @ 2008-10-17 18:30 UTC (permalink / raw)
  To: Andrew Morton; +Cc: mingo, linux-kernel, stable

On Thu, Oct 16, 2008 at 04:11:45PM -0700, Andrew Morton wrote:
> On Thu, 16 Oct 2008 07:58:08 -0500
> Dean Nelson <dcn@sgi.com> wrote:
> 
> > If the member 'name' of the irq_desc structure happens to point to a character
> > string that is resident within a kernel module, problems insue if that module
> > is rmmod'd (at which time dynamic_irq_cleanup() is called) and then later
> > show_interrupts() is called by someone.
> 
> It would be nice to spell out what the "problems" are.

I'm working on a drivers/misc/sgi-xp patchset for SGI's UV system that will
call uv_setup_irq() and uv_teardown_irq(), both of which were submitted as a
patch (Subject: [PATCH] x86, UV: add uv_setup_irq() and uv_teardown_irq()
functions v.3) which has been applied to Ingo's tip/x86/uv.

The first argument to uv_setup_irq() is a pointer to the irq_name to be
stored in irq_desc.name. For XPC, it points to a string resident in the
kernel module.

When I rmmod xpc, uv_teardown_irq() calls destroy_irq() which in turn
calls dynamic_irq_cleanup().

In arch/x86/kernel/irq_64.c, show_interrupts() references the 'name' member
in the following manner:

	seq_printf(p, "-%-8s", irq_desc[i].name);

In my case, show_interrupts was called by the irq balancer and the end result
was an Oops. Note that this can occur later than the rmmod such that the
two events do not appear related.

> 
> Because we should work out whether this should be backported into
> -stable.  And if so, how far back it should go.

I don't have an answer for this.


^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2008-10-17 18:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-10-16 12:58 [PATCH] NULL struct irq_desc's member 'name' in dynamic_irq_cleanup() Dean Nelson
2008-10-16 23:11 ` Andrew Morton
2008-10-17 18:30   ` Dean Nelson

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).