2010-07-09 11:10:35

by Thomas Renninger

[permalink] [raw]
Subject: Dynamic Debug lib: Fix memory corruption for specific module declarations

From: Jason Baron <[email protected]>

Make sure we properly call ddebug_remove_module() when a module fails to
load. In addition, pass the pointer to the "debug table", to both
ddebug_add_module(), and ddebug_remove_module() so that we can uniquely
identify each set of debug statements. In this way even modules with the
same name can be properly identified and removed.

Kernel bug reference:
http://bugzilla.kernel.org/show_bug.cgi?id=16330

By trenn (for stable people):
Not sure for how long this bug exists (always?), it nearly patches
fine for a 2.6.32 kernel...

Signed-off-by: Jason Baron <[email protected]>
Reported-by: Thomas Renninger <[email protected]>
Tested-by: Thomas Renninger <[email protected]>
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]
CC: [email protected]

---
include/linux/dynamic_debug.h | 7 ++-----
include/linux/module.h | 4 ++++
kernel/module.c | 10 +++++++---
lib/dynamic_debug.c | 4 ++--
4 files changed, 15 insertions(+), 10 deletions(-)

diff --git a/include/linux/dynamic_debug.h b/include/linux/dynamic_debug.h
index b3cd4de..a5c133e 100644
--- a/include/linux/dynamic_debug.h
+++ b/include/linux/dynamic_debug.h
@@ -40,7 +40,7 @@ int ddebug_add_module(struct _ddebug *tab, unsigned int n,
const char *modname);

#if defined(CONFIG_DYNAMIC_DEBUG)
-extern int ddebug_remove_module(char *mod_name);
+extern int ddebug_remove_module(struct _ddebug *tab, char *mod_name);

#define __dynamic_dbg_enabled(dd) ({ \
int __ret = 0; \
@@ -73,10 +73,7 @@ extern int ddebug_remove_module(char *mod_name);

#else

-static inline int ddebug_remove_module(char *mod)
-{
- return 0;
-}
+#define ddebug_remove_module(tab, name) do {} while (0)

#define dynamic_pr_debug(fmt, ...) \
do { if (0) printk(KERN_DEBUG pr_fmt(fmt), ##__VA_ARGS__); } while (0)
diff --git a/include/linux/module.h b/include/linux/module.h
index 8a6b9fd..97ce090 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -387,6 +387,10 @@ struct module
ctor_fn_t *ctors;
unsigned int num_ctors;
#endif
+
+#ifdef CONFIG_DYNAMIC_DEBUG
+ struct _ddebug *ddebug;
+#endif
};
#ifndef MODULE_ARCH_INIT
#define MODULE_ARCH_INIT {}
diff --git a/kernel/module.c b/kernel/module.c
index 8c6b428..16bb044 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,8 @@ static void free_module(struct module *mod)
remove_sect_attrs(mod);
mod_kobject_remove(mod);

+ ddebug_remove_module(mod->ddebug, mod->name);
+
/* Arch-specific cleanup. */
module_arch_cleanup(mod);

@@ -2053,12 +2054,14 @@ static inline void add_kallsyms(struct module *mod,
}
#endif /* CONFIG_KALLSYMS */

-static void dynamic_debug_setup(struct _ddebug *debug, unsigned int num)
+static void dynamic_debug_setup(struct _ddebug *debug, unsigned int num,
+ struct module *mod)
{
#ifdef CONFIG_DYNAMIC_DEBUG
if (ddebug_add_module(debug, num, debug->modname))
printk(KERN_ERR "dynamic debug error adding module: %s\n",
debug->modname);
+ mod->ddebug = debug;
#endif
}

@@ -2483,7 +2486,7 @@ static noinline struct module *load_module(void __user *umod,
debug = section_objs(hdr, sechdrs, secstrings, "__verbose",
sizeof(*debug), &num_debug);
if (debug)
- dynamic_debug_setup(debug, num_debug);
+ dynamic_debug_setup(debug, num_debug, mod);
}

err = module_finalize(hdr, sechdrs, mod);
@@ -2562,6 +2565,7 @@ static noinline struct module *load_module(void __user *umod,
synchronize_sched();
module_arch_cleanup(mod);
cleanup:
+ ddebug_remove_module(mod->ddebug, mod->name);
free_modinfo(mod);
module_unload_free(mod);
#if defined(CONFIG_MODULE_UNLOAD)
diff --git a/lib/dynamic_debug.c b/lib/dynamic_debug.c
index 3df8eb1..7d66180 100644
--- a/lib/dynamic_debug.c
+++ b/lib/dynamic_debug.c
@@ -692,7 +692,7 @@ static void ddebug_table_free(struct ddebug_table *dt)
* Called in response to a module being unloaded. Removes
* any ddebug_table's which point at the module.
*/
-int ddebug_remove_module(char *mod_name)
+int ddebug_remove_module(struct _ddebug *tab, char *mod_name)
{
struct ddebug_table *dt, *nextdt;
int ret = -ENOENT;
@@ -703,7 +703,7 @@ int ddebug_remove_module(char *mod_name)

mutex_lock(&ddebug_lock);
list_for_each_entry_safe(dt, nextdt, &ddebug_tables, link) {
- if (!strcmp(dt->mod_name, mod_name)) {
+ if (!strcmp(dt->mod_name, mod_name) && (tab == dt->ddebugs)) {
ddebug_table_free(dt);
ret = 0;
}
--
1.7.1


2010-07-09 23:58:49

by Andrew Morton

[permalink] [raw]
Subject: Re: Dynamic Debug lib: Fix memory corruption for specific module declarations

On Fri, 9 Jul 2010 13:10:42 +0200
Thomas Renninger <[email protected]> wrote:

> From: Jason Baron <[email protected]>

You forgot to cc Jason.

> Make sure we properly call ddebug_remove_module() when a module fails to
> load. In addition, pass the pointer to the "debug table", to both
> ddebug_add_module(), and ddebug_remove_module() so that we can uniquely
> identify each set of debug statements. In this way even modules with the
> same name can be properly identified and removed.
>
> Kernel bug reference:
> http://bugzilla.kernel.org/show_bug.cgi?id=16330
>
> By trenn (for stable people):
> Not sure for how long this bug exists (always?), it nearly patches
> fine for a 2.6.32 kernel...

It's unclear what kernel this is against. It applies quite badly to
mainline. I fixed all that up then lost the result :( It doesn't apply
cleanly to 2.6.34 either.

I was going to steal your changelog and add it to Jason's patch but
Jason's patch generates a reject storm against mainline too.

Giving up.