[PATCH] dynamic debug: move ddebug_remove_module() down into free_module()

[PATCH] dynamic debug: move ddebug_remove_module() down into free_module()

[Jason Baron]

move the call to ddebug_remove_module() down into free_module(). In this way
it should be called from all error paths. Currently, we are missing the
remove if the module init routine fails.

Signed-off-by: Jason Baron <jbaron@redhat.com>
Reported-by: Thomas Renninger <trenn@suse.de>
Tested-by: Thomas Renninger <trenn@suse.de>
CC: akpm@linux-foundation.org
CC: andi@firstfloor.org
CC: hare@suse.de
CC: trenn@suse.de
CC: yehuda@hq.newdream.net
CC: linux-kernel@vger.kernel.org
---
 kernel/module.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/module.c b/kernel/module.c
index 5d2d281..6c56282 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -787,7 +787,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
 
 	/* Store the name of the last unloaded module for diagnostic purposes */
 	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));
-	ddebug_remove_module(mod->name);
 
 	free_module(mod);
 	return 0;
@@ -1550,6 +1549,9 @@ static void free_module(struct module *mod)
 	remove_sect_attrs(mod);
 	mod_kobject_remove(mod);
 
+	/* Remove dynamic debug info */
+	ddebug_remove_module(mod->name);
+
 	/* Arch-specific cleanup. */
 	module_arch_cleanup(mod);
 
-- 
1.7.1

Re: [PATCH] dynamic debug: move ddebug_remove_module() down into free_module()

[Andrew Morton]

On Wed, 14 Jul 2010 16:00:57 -0400
Jason Baron <jbaron@redhat.com> wrote:

> move the call to ddebug_remove_module() down into free_module(). In this way
> it should be called from all error paths. Currently, we are missing the
> remove if the module init routine fails.
> 
> Signed-off-by: Jason Baron <jbaron@redhat.com>
> Reported-by: Thomas Renninger <trenn@suse.de>
> Tested-by: Thomas Renninger <trenn@suse.de>
> CC: akpm@linux-foundation.org
> CC: andi@firstfloor.org
> CC: hare@suse.de
> CC: trenn@suse.de
> CC: yehuda@hq.newdream.net
> CC: linux-kernel@vger.kernel.org

Crappy changelog doesn't describe the bug we're fixing.  It should.

It's

	echo "file ec.c +p" >/sys/kernel/debug/dynamic_debug/control

causes an oops, yes?

> diff --git a/kernel/module.c b/kernel/module.c
> index 5d2d281..6c56282 100644
> --- a/kernel/module.c
> +++ b/kernel/module.c
> @@ -787,7 +787,6 @@ SYSCALL_DEFINE2(delete_module, const char __user *, name_user,
>  
>  	/* Store the name of the last unloaded module for diagnostic purposes */
>  	strlcpy(last_unloaded_module, mod->name, sizeof(last_unloaded_module));
> -	ddebug_remove_module(mod->name);
>  
>  	free_module(mod);
>  	return 0;
> @@ -1550,6 +1549,9 @@ static void free_module(struct module *mod)
>  	remove_sect_attrs(mod);
>  	mod_kobject_remove(mod);
>  
> +	/* Remove dynamic debug info */
> +	ddebug_remove_module(mod->name);
> +
>  	/* Arch-specific cleanup. */
>  	module_arch_cleanup(mod);

Should we backport this into 2.6.34.x?  Earlier?

Re: [PATCH] dynamic debug: move ddebug_remove_module() down into free_module()

[Thomas Renninger]

On Wednesday 21 July 2010 23:34:23 Andrew Morton wrote:
> On Wed, 14 Jul 2010 16:00:57 -0400
> Jason Baron <jbaron@redhat.com> wrote:
> 
> > move the call to ddebug_remove_module() down into free_module(). In this way
> > it should be called from all error paths. Currently, we are missing the
> > remove if the module init routine fails.
> > 
> > Signed-off-by: Jason Baron <jbaron@redhat.com>
> > Reported-by: Thomas Renninger <trenn@suse.de>
> > Tested-by: Thomas Renninger <trenn@suse.de>
> > CC: akpm@linux-foundation.org
> > CC: andi@firstfloor.org
> > CC: hare@suse.de
> > CC: trenn@suse.de
> > CC: yehuda@hq.newdream.net
> > CC: linux-kernel@vger.kernel.org
> 
> Crappy changelog doesn't describe the bug we're fixing.  It should.
> 
> It's
> 
> 	echo "file ec.c +p" >/sys/kernel/debug/dynamic_debug/control

> causes an oops, yes?
AFAIK the query written doesn't matter.
It's about the declaration of specific modules (see below).
If you compile out (a) specific module(s), above query might even
work.
 
... 
> Should we backport this into 2.6.34.x?  Earlier?
We saw oopses in 2.6.32 due to a module without an exit function
and overwriting the pr_fmt macro (not 100% sure whether both
conditions must be met, according to Hannes' test yes).
I expect the patch should go at least back to this one.
I can give it a try then whether this also fixes this condition.

      Thomas