[patch 2/2] track and print last unloaded module in the oops trace

[patch 2/2] track and print last unloaded module in the oops trace

[Arjan van de Ven]

Subject: track and print last unloaded module in the oops trace
From: Arjan van de Ven <arjan@linux.intel.com>
CC: rusty@rustcorp.com.au
CC: ak@suse.de
CC: mingo@elte.hu

Based on a suggestion from Andi: 
In various cases, the unload of a module may leave some bad state around
that causes a kernel crash AFTER a module is unloaded; and it's then hard
to find which module caused that. 

This patch tracks the last unloaded module, and prints this as part of the
module list in the oops trace.

Right now, only the last 1 module is tracked; I expect that this is enough
for the vast majority of cases where this information matters; if it turns
out that tracking more is important, we can always extend it to that.

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

---
 kernel/module.c |    6 ++++++
 1 file changed, 6 insertions(+)

Index: linux-2.6.24-rc6/kernel/module.c
===================================================================
--- linux-2.6.24-rc6.orig/kernel/module.c
+++ linux-2.6.24-rc6/kernel/module.c
@@ -655,6 +655,8 @@ static void wait_for_zero_refcount(struc
 	mutex_lock(&module_mutex);
 }
 
+static char last_unloaded_module[MODULE_NAME_LEN+1];
+
 asmlinkage long
 sys_delete_module(const char __user *name_user, unsigned int flags)
 {
@@ -721,6 +723,8 @@ sys_delete_module(const char __user *nam
 		mod->exit();
 		mutex_lock(&module_mutex);
 	}
+	/* Store the name of the last unloaded module for diagnostic purposes */
+	sprintf(last_unloaded_module, mod->name);
 	free_module(mod);
 
  out:
@@ -2501,6 +2505,8 @@ void print_modules(void)
 	printk("Modules linked in:");
 	list_for_each_entry(mod, &modules, list)
 		printk(" %s%s", mod->name, module_flags(mod, buf));
+	if (last_unloaded_module[0])
+		printk(" [last unloaded: %s]", last_unloaded_module);
 	printk("\n");
 }
 

-- 
If you want to reach me at my work email, use arjan@linux.intel.com
For development, discussion and tips for power savings, 
visit http://www.lesswatts.org

Re: [patch 2/2] track and print last unloaded module in the oops trace

[Rusty Russell]

On Monday 07 January 2008 10:19:46 Arjan van de Ven wrote:
> Subject: track and print last unloaded module in the oops trace
> From: Arjan van de Ven <arjan@linux.intel.com>
> CC: rusty@rustcorp.com.au
> CC: ak@suse.de
> CC: mingo@elte.hu
>
> Based on a suggestion from Andi:
> In various cases, the unload of a module may leave some bad state around
> that causes a kernel crash AFTER a module is unloaded; and it's then hard
> to find which module caused that.
>
> This patch tracks the last unloaded module, and prints this as part of the
> module list in the oops trace.
>
> Right now, only the last 1 module is tracked; I expect that this is enough
> for the vast majority of cases where this information matters; if it turns
> out that tracking more is important, we can always extend it to that.
>
> Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>

Thanks, applied.

Rusty.

Re: [patch 2/2] track and print last unloaded module in the oops trace

[Ingo Molnar]

* Rusty Russell <rusty@rustcorp.com.au> wrote:

> > Right now, only the last 1 module is tracked; I expect that this is 
> > enough for the vast majority of cases where this information 
> > matters; if it turns out that tracking more is important, we can 
> > always extend it to that.
> >
> > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> 
> Thanks, applied.

needs the fix below.

	Ingo

-------------->
Subject: track and print last unloaded module in the oops trace, fix
From: Ingo Molnar <mingo@elte.hu>

fix:

kernel/module.c: In function 'print_modules':
kernel/module.c:2508: error: 'last_unloaded_module' undeclared (first use in this function)
kernel/module.c:2508: error: (Each undeclared identifier is reported only once
kernel/module.c:2508: error: for each function it appears in.)

Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 kernel/module.c |    4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

Index: linux/kernel/module.c
===================================================================
--- linux.orig/kernel/module.c
+++ linux/kernel/module.c
@@ -498,6 +498,8 @@ static struct module_attribute modinfo_#
 MODINFO_ATTR(version);
 MODINFO_ATTR(srcversion);
 
+static char last_unloaded_module[MODULE_NAME_LEN+1];
+
 #ifdef CONFIG_MODULE_UNLOAD
 /* Init the unload section of the module. */
 static void module_unload_init(struct module *mod)
@@ -655,8 +657,6 @@ static void wait_for_zero_refcount(struc
 	mutex_lock(&module_mutex);
 }
 
-static char last_unloaded_module[MODULE_NAME_LEN+1];
-
 asmlinkage long
 sys_delete_module(const char __user *name_user, unsigned int flags)
 {

Re: [patch 2/2] track and print last unloaded module in the oops trace

[Rusty Russell]

On Tuesday 08 January 2008 22:23:31 Ingo Molnar wrote:
> * Rusty Russell <rusty@rustcorp.com.au> wrote:
> > > Right now, only the last 1 module is tracked; I expect that this is
> > > enough for the vast majority of cases where this information
> > > matters; if it turns out that tracking more is important, we can
> > > always extend it to that.
> > >
> > > Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
> >
> > Thanks, applied.
>
> needs the fix below.

Thanks, I rolled that in.

Cheers,
Rusty.

Re: [patch 2/2] track and print last unloaded module in the oops trace

[DM]

On Jan 7, 2008 12:19 AM, Arjan van de Ven <arjan@infradead.org> wrote:
[...]
> This patch tracks the last unloaded module, and prints this as part of the
> module list in the oops trace.
>
[...]
>        }
> +       /* Store the name of the last unloaded module for diagnostic purposes */
> +       sprintf(last_unloaded_module, mod->name);
>        free_module(mod);
>

Why use sprintf? If a module name contains the % character we could
overflow the buffer. Or is module-unloading root-only and we don't
care?

Best regards,
dm.n9107

Re: [patch 2/2] track and print last unloaded module in the oops trace

[Arjan van de Ven]

On Tue, 8 Jan 2008 14:51:37 +0100
DM <dm.n9107@gmail.com> wrote:

> On Jan 7, 2008 12:19 AM, Arjan van de Ven <arjan@infradead.org> wrote:
> [...]
> > This patch tracks the last unloaded module, and prints this as part
> > of the module list in the oops trace.
> >
> [...]
> >        }
> > +       /* Store the name of the last unloaded module for
> > diagnostic purposes */
> > +       sprintf(last_unloaded_module, mod->name);
> >        free_module(mod);
> >
> 
> Why use sprintf? If a module name contains the % character we could
> overflow the buffer. Or is module-unloading root-only and we don't
> care?

module loading isn't just root only; the name comes from an already loaded module.
If you can load kernel modules of your choice you own the entire kernel already anway

Re: [patch 2/2] track and print last unloaded module in the oops trace

[DM]

On Jan 8, 2008 3:26 PM, Arjan van de Ven <arjan@infradead.org> wrote:
> >
> > Why use sprintf? If a module name contains the % character we could
> > overflow the buffer. Or is module-unloading root-only and we don't
> > care?
>
> module loading isn't just root only; the name comes from an already loaded module.
> If you can load kernel modules of your choice you own the entire kernel already anway
>

Still, strcpy seems like a better choice IMHO.

/DM

Re: [patch 2/2] track and print last unloaded module in the oops trace

[Ingo Molnar]

* DM <dm.n9107@gmail.com> wrote:

> On Jan 8, 2008 3:26 PM, Arjan van de Ven <arjan@infradead.org> wrote:
> > >
> > > Why use sprintf? If a module name contains the % character we could
> > > overflow the buffer. Or is module-unloading root-only and we don't
> > > care?
> >
> > module loading isn't just root only; the name comes from an already loaded module.
> > If you can load kernel modules of your choice you own the entire kernel already anway
> >
> 
> Still, strcpy seems like a better choice IMHO.

agreed, this just isnt obvious IMO:

+       sprintf(last_unloaded_module, mod->name);

	Ingo

Re: [patch 2/2] track and print last unloaded module in the oops trace

[Rusty Russell]

On Wednesday 09 January 2008 03:18:46 Ingo Molnar wrote:
> * DM <dm.n9107@gmail.com> wrote:
> > On Jan 8, 2008 3:26 PM, Arjan van de Ven <arjan@infradead.org> wrote:
> > > > Why use sprintf? If a module name contains the % character we could
> > > > overflow the buffer. Or is module-unloading root-only and we don't
> > > > care?
> > >
> > > module loading isn't just root only; the name comes from an already
> > > loaded module. If you can load kernel modules of your choice you own
> > > the entire kernel already anway
> >
> > Still, strcpy seems like a better choice IMHO.
>
> agreed, this just isnt obvious IMO:
>
> +       sprintf(last_unloaded_module, mod->name);
>
> 	Ingo

Yes, I've changed it to:
+	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));

In my tree.

Thanks,
Rusty.