Problem Description: a module's exported functions can be called before before they are properly initialized by the module_init function. Steps to reproduce: write a module that exports functions that require initialization by the module_init function to work correctly. E.g. spin lock variables are no longer allowed to be initialized by C initializers of the module but only by spin_lock_init that can be called by the module_init function. If an exported function calls spin_lock before it is initialized, it deadlocks.
Reply-To: akpm@linux-foundation.org On Wed, 29 Aug 2007 11:33:06 -0700 (PDT) bugme-daemon@bugzilla.kernel.org wrote: > http://bugzilla.kernel.org/show_bug.cgi?id=8957 > > Summary: Exported functions and variables should not be reachable > by the outside of the module until module_init finishes > Product: Other > Version: 2.5 > KernelVersion: 2.6.23-rc4 > Platform: All > OS/Version: Linux > Tree: Mainline > Status: NEW > Severity: normal > Priority: P1 > Component: Modules > AssignedTo: other_modules@kernel-bugs.osdl.org > ReportedBy: mattilinnanvuori@yahoo.com > > > Problem Description: a module's exported functions can be called before > before > they are properly initialized by the module_init function. > > Steps to reproduce: write a module that exports functions that require > initialization by the module_init function to work correctly. > > E.g. spin lock variables are no longer allowed to be initialized by C > initializers of the module but only by spin_lock_init that can be called by > the > module_init function. If an exported function calls spin_lock before it is > initialized, it deadlocks. > ooh, nice bug ;)
Andrew Morton wrote: > On Wed, 29 Aug 2007 11:33:06 -0700 (PDT) bugme-daemon@bugzilla.kernel.org > wrote: > >> http://bugzilla.kernel.org/show_bug.cgi?id=8957 >> >> Summary: Exported functions and variables should not be reachable >> by the outside of the module until module_init finishes >> Product: Other >> Version: 2.5 >> KernelVersion: 2.6.23-rc4 >> Platform: All >> OS/Version: Linux >> Tree: Mainline >> Status: NEW >> Severity: normal >> Priority: P1 >> Component: Modules >> AssignedTo: other_modules@kernel-bugs.osdl.org >> ReportedBy: mattilinnanvuori@yahoo.com >> >> >> Problem Description: a module's exported functions can be called before >> before >> they are properly initialized by the module_init function. >> >> Steps to reproduce: write a module that exports functions that require >> initialization by the module_init function to work correctly. >> >> E.g. spin lock variables are no longer allowed to be initialized by C >> initializers of the module but only by spin_lock_init that can be called by >> the >> module_init function. If an exported function calls spin_lock before it is >> initialized, it deadlocks. >> > > ooh, nice bug ;) Under what circumstances is this actually happening? What are these functions that are being called? Normally things are set up such that this isn't a problem, i.e. if module A depends on module B, module A can't load until module B is finished loading.
Reply-To: akpm@linux-foundation.org On Wed, 29 Aug 2007 19:33:48 -0600 Robert Hancock <hancockr@shaw.ca> wrote: > Andrew Morton wrote: > > On Wed, 29 Aug 2007 11:33:06 -0700 (PDT) bugme-daemon@bugzilla.kernel.org > wrote: > > > >> http://bugzilla.kernel.org/show_bug.cgi?id=8957 > >> > >> Summary: Exported functions and variables should not be > reachable > >> by the outside of the module until module_init > finishes > >> Product: Other > >> Version: 2.5 > >> KernelVersion: 2.6.23-rc4 > >> Platform: All > >> OS/Version: Linux > >> Tree: Mainline > >> Status: NEW > >> Severity: normal > >> Priority: P1 > >> Component: Modules > >> AssignedTo: other_modules@kernel-bugs.osdl.org > >> ReportedBy: mattilinnanvuori@yahoo.com > >> > >> > >> Problem Description: a module's exported functions can be called before > before > >> they are properly initialized by the module_init function. > >> > >> Steps to reproduce: write a module that exports functions that require > >> initialization by the module_init function to work correctly. > >> > >> E.g. spin lock variables are no longer allowed to be initialized by C > >> initializers of the module but only by spin_lock_init that can be called > by the > >> module_init function. If an exported function calls spin_lock before it is > >> initialized, it deadlocks. > >> > > > > ooh, nice bug ;) > > Under what circumstances is this actually happening? What are these > functions that are being called? > > Normally things are set up such that this isn't a problem, i.e. if > module A depends on module B, module A can't load until module B is > finished loading. > Good point. This thus-far-undescribed module could make its internals externally visible via one of the kernel's many register_foo() interfaces, but it would be a buggy module if it was doing register_foo(my_foo) before my_foo() was ready to be called.
I thought that if module A depends on module B, module A can load and access B's exported symbols before module B is finished with its module_init function.
I thought that the bug might happen when two kernel modules are being loaded. If module A is loaded and its code includes references to functions exported by module B, I thought module A could call those functions before the module_init function of module B has finished. I was not thinking about buggy calls to registering interface functions. I just thought that the kernel should not allow symbols exported by EXPORT_SYMBOLto be visible to other modules before the module_init function is finished. One could code the exported functions so that they could be safely called by anyone while the module_init function is being called but that would be an unnecessary burden for coders. I think that a module should expose its functions and variables only by calling registering interface functions before the module_init function is finished. So I think the design of the kernel modules is flawed if it allows anyone to call exported functions before the module_init function is finished. Heute schon einen Blick in die Zukunft von E-Mails wagen? Versuchen Sie
On Thu, 30 Aug 2007 09:41:41 -0700 (PDT) Matti Linnanvuori <mattilinnanvuori@yahoo.com> wrote: > I thought that the bug might happen when two kernel modules are being > loaded. If module A is loaded and its code includes references to > functions exported by module B, I thought module A could call those > functions before the module_init function of module B has finished. I > was not thinking about buggy calls to registering interface > functions. I just thought that the kernel should not allow symbols > exported by EXPORT_SYMBOLto be visible to other modules before the > module_init function is finished. One could code the exported > functions so that they could be safely called by anyone while the > module_init function is being called but that would be an unnecessary > burden for coders. I think that a module should expose its functions > and variables only by calling registering interface functions before > the module_init function is finished. So I think the design of the > kernel modules is flawed if it allows anyone to call exported > functions before the module_init function is finished. > have you seen this? module loading is pretty much serialized, so that no 2 modules are being loaded in parallel...
On Wed, 29 Aug 2007, Robert Hancock wrote: > Andrew Morton wrote: > > On Wed, 29 Aug 2007 11:33:06 -0700 (PDT) bugme-daemon@bugzilla.kernel.org > > wrote: > > > > > http://bugzilla.kernel.org/show_bug.cgi?id=8957 > > > > > > Summary: Exported functions and variables should not be > > > reachable > > > by the outside of the module until module_init > > > finishes > > > Product: Other > > > Version: 2.5 > > > KernelVersion: 2.6.23-rc4 > > > Platform: All > > > OS/Version: Linux > > > Tree: Mainline > > > >> Status: NEW > > > Severity: normal > > > Priority: P1 > > > Component: Modules > > > AssignedTo: other_modules@kernel-bugs.osdl.org > > > ReportedBy: mattilinnanvuori@yahoo.com > > > > > > > > > Problem Description: a module's exported functions can be called before > > > before > > > they are properly initialized by the module_init function. > > > > > > Steps to reproduce: write a module that exports functions that require > > > initialization by the module_init function to work correctly. > > > > > > E.g. spin lock variables are no longer allowed to be initialized by C > > > initializers of the module but only by spin_lock_init that can be called > > > by the > > > module_init function. If an exported function calls spin_lock before it > is > > > initialized, it deadlocks. Hmm, can you post some sample code / sample module to reproduce this? I don't think exported symbols can be resolved till our module finishes loading + initializing. There's a whole lot of dancing in the libusual module precisely to cope with this behaviour. > > ooh, nice bug ;) > > Under what circumstances is this actually happening? What are these functions > that are being called? > > Normally things are set up such that this isn't a problem, i.e. if module A > depends on module B, module A can't load until module B is finished loading. See drivers/usb/storage/libusual.c -- pretty unusual goings on there :-) It needs to request_module() another module (that will reference our exported symbols). To cope with the fact that our exported modules _cannot_ be resolved till we finish loading, it uses semaphore-used-as- completion-handler kludge to let another "probe" kthread know when our module_init() function is done, so that it can proceed to request_module() the other module. Interestingly, the kthread that request_module()s the other module is spawned from the struct usb_driver ->probe() function (not an exported function) and the claim there is that (1) usb_driver ->probe() can be called out without the module_init() of libusual having finished, and, (2) the newly requested module's loading will fail because it cannot resolve libusual's exported symbols till we have finished module_init(). Satyam
I thought I had seen that bug. Module init function execution does not seem serialized enough, so the init function of one module seems to be able to be called in parallel with several other modules in turn being loaded, executing their init functions and even becoming live first class citizens. Function sys_init_module in Linux 2.6.22.x and 2.6.23-rc4 kernel/module.c does not hold module_mutex when executing the init functions of the modules. __________________________________ Alles was der Gesundheit und Entspannung dient. BE A BETTER MEDIZINMANN! www.yahoo.de/clever
Hi Andrew, On Wed, 29 Aug 2007, Andrew Morton wrote: > On Wed, 29 Aug 2007 19:33:48 -0600 Robert Hancock <hancockr@shaw.ca> wrote: > > > Andrew Morton wrote: > > > On Wed, 29 Aug 2007 11:33:06 -0700 (PDT) bugme-daemon@bugzilla.kernel.org > wrote: > > > > > >> http://bugzilla.kernel.org/show_bug.cgi?id=8957 > > >> > > >> Summary: Exported functions and variables should not be > reachable > > >> by the outside of the module until module_init > finishes > > >> Product: Other > > >> Version: 2.5 > > >> KernelVersion: 2.6.23-rc4 > > >> Platform: All > > >> OS/Version: Linux > > >> Tree: Mainline > > >> Status: NEW > > >> Severity: normal > > >> Priority: P1 > > >> Component: Modules > > >> AssignedTo: other_modules@kernel-bugs.osdl.org > > >> ReportedBy: mattilinnanvuori@yahoo.com > > >> > > >> > > >> Problem Description: a module's exported functions can be called before > before > > >> they are properly initialized by the module_init function. > > >> > > >> Steps to reproduce: write a module that exports functions that require > > >> initialization by the module_init function to work correctly. > > >> > > >> E.g. spin lock variables are no longer allowed to be initialized by C > > >> initializers of the module but only by spin_lock_init that can be called > by the > > >> module_init function. If an exported function calls spin_lock before it > is > > >> initialized, it deadlocks. > > > > > > ooh, nice bug ;) > > > > Under what circumstances is this actually happening? What are these > > functions that are being called? > > > > Normally things are set up such that this isn't a problem, i.e. if > > module A depends on module B, module A can't load until module B is > > finished loading. > > Good point. > > This thus-far-undescribed module could make its internals externally > visible via one of the kernel's many register_foo() interfaces, What you're saying is a plausible problem, but note that it is quite a completely different issue to what Matti Linnanvuori suggested in the original bug report. The report was about module B (which depends on module A, because it references symbol exported by module A) being able to call a function (or access data) /exported/ by module A _without_ the module_init() function of module A having finished completely (and hence the possibility of accessing uninitialized data etc). But this is not possible -- see the last reply to Matti. You're referring to is a module implementing an (possibly un-exported) function that refers to module-local data, and registering that function (say through a notifier_block) _before_ initializing_ the data used by that function. But ... > but it > would be a buggy module if it was doing register_foo(my_foo) before > my_foo() was ready to be called. ... exactly. That module is the buggy culprit here, nothing wrong with the kernel's core module code. [ BTW I suspect there /are/ modules out there that get this register_foo() ordering wrong in their module_init functions. Even more widespread (as I have noticed) is the sad habit of modules to not unregister_foo() their stuff (in the module_exit function) in the exact reverse order of the register_foo() calls made during module_init. This can clearly lead to oopsen, but the only reason why we don't see them frequently is because the module_init and module_exit codepaths are rarely ever executed at runtime, and even more rarely concurrently with other stuff that's using the module. ] Satyam
Hi Matti, On Thu, 30 Aug 2007, Matti Linnanvuori wrote: > > I thought I had seen that bug. Module init function execution does not > seem serialized enough, so the init function of one module seems to be > able to be called in parallel with several other modules in turn being > loaded, executing their init functions and even becoming live first > class citizens. > Function sys_init_module in > Linux 2.6.22.x and 2.6.23-rc4 kernel/module.c does not hold module_mutex > when executing the init functions of the modules. Sure, there's nothing to prevent one module's module_init() from being concurrently executed with the module_init() of another -- but that's OK. Anyway, I tested this out with: Module A ======== /***** mod_a.c *****/ #include <linux/kernel.h> #include <linux/init.h> #include <linux/delay.h> #include <linux/module.h> #include <linux/spinlock.h> static spinlock_t mylock; static void mod_a_func(void) { spin_lock(&mylock); printk(KERN_INFO "inside mod_a_func\n"); spin_unlock(&mylock); } EXPORT_SYMBOL(mod_a_func); static int __init mod_a_init(void) { printk(KERN_INFO "module_init begin ..."); ssleep(10); spin_lock_init(&mylock); printk(KERN_INFO "... module_init done\n"); return 0; } static void __exit mod_a_exit(void) { printk(KERN_INFO "exiting\n"); } module_init(mod_a_init); module_exit(mod_a_exit); MODULE_LICENSE("GPL"); /***** mod_a.c *****/ Module B ======== /***** mod_b.c *****/ #include <linux/kernel.h> #include <linux/init.h> #include <linux/module.h> extern void mod_a_func(void); static int __init mod_b_init(void) { mod_a_func(); return 0; } static void __exit mod_b_exit(void) { } module_init(mod_b_init); module_exit(mod_b_exit); MODULE_LICENSE("GPL"); /***** mod_b.c *****/ Test result: ============ Module B refuses to load (could not resolve the symbol mod_a_func) unless the module_init() of Module A has finished completely. The following log was obtained by running "insmod ./mod_b.ko" repeatedly on one xterm, while another xterm was then used to do "insmod ./mod_a.ko": mod_b: Unknown symbol mod_a_func mod_b: Unknown symbol mod_a_func module_init begin ...<4>mod_b: Unknown symbol mod_a_func mod_b: Unknown symbol mod_a_func last message repeated 4 times ... module_init done inside mod_a_func So the title of your bug report "Exported functions and variables should not be reachable by the outside of the module until module_init finishes" sounds to be already true to me, so let us know if bugzilla #8957 can be closed or not ;-) Thanks, Satyam
It seems to me that kernel/module.c allows the whole kernel to use exported symbols during the execution of the init function if they are weak: /* Ok if weak. */ if (ELF_ST_BIND(sym[i].st_info) == STB_WEAK) break; That seems a possible way to produce the scenario of this so-called bug. ________ Yahoo! Clever: Stellen Sie Fragen und finden Sie Antworten. Teilen Sie Ihr Wissen. www.yahoo.de/clever
On Fri, 31 Aug 2007 10:14:27 -0700 (PDT) Matti Linnanvuori <mattilinnanvuori@yahoo.com> wrote: > It seems to me that kernel/module.c allows the whole kernel to use > exported symbols during the execution of the init function if they > are weak: /* Ok if weak. */ if (ELF_ST_BIND(sym[i].st_info) == > STB_WEAK) break; > That seems a possible way to produce the scenario of this so-called > bug. if you export weak symbols in your own module AND you have some piece of kernel that will start using the symbol (which means the kernel already needs to know about it) AND you don't know what you're doing you get shot in the foot... BFD maybe if you have such a case you should publish the source code of that case so that we can suggest alternative approaches of what you're trying to do..
On Fri, 31 Aug 2007, Matti Linnanvuori wrote: > > It seems to me that kernel/module.c allows the whole kernel to use > exported symbols during the execution of the init function if they are > weak: > /* Ok if weak. */ > if (ELF_ST_BIND(sym[i].st_info) == STB_WEAK) > break; > That seems a possible way to produce the scenario of this so-called bug. No, even that won't reproduce the bug you're talking about, and you clearly don't know how weak symbols are supposed to work / be used :-) simplify_symbols() -> resolve_symbol() is called to resolve /external/ symbols that the module-being-loaded references, and error out in case no such (global, exported) symbol was currently found. So the "sym[i]" there refers to the (as yet unresolved) symbol referenced in the _dependent module B_, that it sees exported as a weak symbol (probably because marked as such in some header prototype). That check is to support usage where we still allow B to load without A being loaded, because it's somehow ensured that B will never call that function at runtime unless it is available ... something like: extern void mod_a_func(void) __attribute__((weak)); static int __init mod_b_init(void) { if (mod_a_func) mod_a_func(); else { /* some remedial action */ printk(KERN_INFO "own little mod_a_func fallback\n"); } return 0; } Try running the same test I described in previous post with this change. Moreover, failure to check (mod_a_func) will cause an _oops_, and *not* the "module A's exported function being called even when module_init() has not finished" issue that you've complained about. So things look alright to me on this front, the bug has been rightly rejected as invalid. And as Arjan pointed out, if you really saw such an issue, please post some code instead, so that we can have a look. Thanks, Satyam
This is a kernel/module.c Linux 2.6.23-rc5 documentation patch that adds more explanation and corrects a spelling error. --- kernel/module.c 2007-09-01 14:15:06.131101500 +0300 +++ patched-kernel/module.c 2007-09-01 14:15:27.502780500 +0300 @@ -80,7 +80,8 @@ int unregister_module_notifier(struct no } EXPORT_SYMBOL(unregister_module_notifier); -/* We require a truly strong try_module_get() */ +/* We require a truly strong try_module_get(): 0 means failure due to + ongoing or failed initialization etc. */ static inline int strong_try_module_get(struct module *mod) { if (mod && mod->state == MODULE_STATE_COMING) @@ -957,7 +958,8 @@ static unsigned long resolve_symbol(Elf_ ret = __find_symbol(name, &owner, &crc, !(mod->taints & TAINT_PROPRIETARY_MODULE)); if (ret) { - /* use_module can fail due to OOM, or module unloading */ + /* use_module can fail due to OOM, or module + initialization or unloading */ if (!check_version(sechdrs, versindex, name, mod, crc) || !use_module(mod, owner)) ret = 0; @@ -1270,7 +1272,7 @@ dup: return ret; } -/* Change all symbols so that sh_value encodes the pointer directly. */ +/* Change all symbols so that st_value encodes the pointer directly. */ static int simplify_symbols(Elf_Shdr *sechdrs, unsigned int symindex, const char *strtab, Signed-off-by: Matti Linnanvuori <mattilinnanvuori@yahoo.com> Wissenswertes f