Messages in this thread Patch in this message | | | Date | Tue, 17 Mar 2009 04:24:36 +0100 | From | Eric Dumazet <> | Subject | Re: [PATCH] module: fix refptr allocation and release order |
| |
Rusty Russell a écrit : > From: Masami Hiramatsu <mhiramat@redhat.com> > > Impact: fix ref-after-free crash on failed module load > > Fix refptr bug: Change refptr allocation and release order not to access a module > data structure pointed by 'mod' after freeing mod->module_core. > This bug will cause kernel panic(e.g. failed to find undefined symbols). > > This bug was reported on systemtap bugzilla. > http://sources.redhat.com/bugzilla/show_bug.cgi?id=9927 > > Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com> > Cc: Eric Dumazet <dada1@cosmosbay.com> > Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
My original patch did not have this problem, because I used a local variable to hold refptr.
http://www.archivum.info/linux-kernel@vger.kernel.org/2008-05/msg07400.html
A simpler patch could just use a local variable again, since we are very late in rc phase ?
From: Masami Hiramatsu <mhiramat@redhat.com>
Impact: fix ref-after-free crash on failed module load
Fix refptr bug: Change refptr release not to access a module data structure pointed by 'mod' after freeing mod->module_core. This bug will cause kernel panic(e.g. failed to find undefined symbols).
This bug was reported on systemtap bugzilla. http://sources.redhat.com/bugzilla/show_bug.cgi?id=9927
Signed-off-by: Masami Hiramatsu <mhiramat@redhat.com> Signed-off-by: Eric Dumazet <dada1@cosmosbay.com> Signed-off-by: Rusty Russell <rusty@rustcorp.com.au>
diff --git a/kernel/module.c b/kernel/module.c index ba22484..4dd1228 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -1874,6 +1874,9 @@ static noinline struct module *load_module(void __user *umod, struct kernel_param *kp; struct module *mod; long err = 0; +#if defined(CONFIG_MODULE_UNLOAD) && defined(CONFIG_SMP) + void *refptr = NULL; +#endif void *percpu = NULL, *ptr = NULL; /* Stops spurious gcc warning */ unsigned long *mseg; mm_segment_t old_fs; @@ -2018,6 +2021,7 @@ static noinline struct module *load_module(void __user *umod, #if defined(CONFIG_MODULE_UNLOAD) && defined(CONFIG_SMP) mod->refptr = percpu_modalloc(sizeof(local_t), __alignof__(local_t), mod->name); + refptr = mod->refptr; if (!mod->refptr) { err = -ENOMEM; goto free_mod; @@ -2292,10 +2296,14 @@ static noinline struct module *load_module(void __user *umod, free_core: module_free(mod, mod->module_core); free_percpu: + /* + * Do not access to mod anymore, it's now freed + */ if (percpu) percpu_modfree(percpu); #if defined(CONFIG_MODULE_UNLOAD) && defined(CONFIG_SMP) - percpu_modfree(mod->refptr); + if (refptr) + percpu_modfree(refptr); #endif free_mod: kfree(args); -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |