Date: Mon, 23 Aug 2010 11:10:04 -0400 From: John Baldwin <jhb@freebsd.org> To: Andriy Gapon <avg@icyb.net.ua> Cc: freebsd-hackers@freebsd.org, Ryan Stone <rysto32@gmail.com> Subject: Re: kld modules remain loaded if MOD_LOAD handler returns an error Message-ID: <201008231110.04552.jhb@freebsd.org> In-Reply-To: <4C728DF4.2060203@icyb.net.ua> References: <AANLkTi=qJ3WZChWg5axnuH8OBMxzd6JrnmfsmXHqPY2Q@mail.gmail.com> <201008230810.04710.jhb@freebsd.org> <4C728DF4.2060203@icyb.net.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Monday, August 23, 2010 11:04:20 am Andriy Gapon wrote: > on 23/08/2010 15:10 John Baldwin said the following: > > On Friday, August 20, 2010 1:13:53 pm Ryan Stone wrote: > >> Consider the following modules: > >> > >> /* first.c */ > >> static int *test; > >> > >> int > >> test_function(void) > >> { > >> return *test; > >> } > >> > >> static int > >> first_modevent(struct module *m, int what, void *arg) > >> { > >> int err = 0; > >> > >> switch (what) { > >> case MOD_LOAD: /* kldload */ > >> test = malloc(sizeof(int), M_TEMP, M_NOWAIT | M_ZERO); > >> if (!test) > >> err = ENOMEM; > >> break; > >> case MOD_UNLOAD: /* kldunload */ > >> break; > >> default: > >> err = EINVAL; > >> break; > >> } > >> return(err); > >> } > >> > >> static moduledata_t first_mod = { > >> "first", > >> first_modevent, > >> NULL > >> }; > >> > >> DECLARE_MODULE(first, first_mod, SI_SUB_KLD, SI_ORDER_ANY); > >> MODULE_VERSION(first, 1); > >> > >> > >> /* second.c */ > >> static int > >> second_modevent(struct module *m, int what, void *arg) > >> { > >> int err = 0; > >> > >> switch (what) { > >> case MOD_LOAD: /* kldload */ > >> test_function(); > >> break; > >> case MOD_UNLOAD: /* kldunload */ > >> break; > >> default: > >> err = EINVAL; > >> break; > >> } > >> return(err); > >> } > >> > >> static moduledata_t second_mod = { > >> "second", > >> second_modevent, > >> NULL > >> }; > >> > >> DECLARE_MODULE(second, second_mod, SI_SUB_KLD, SI_ORDER_ANY); > >> MODULE_DEPEND(second, first, 1, 1, 1); > >> > >> > >> Consider the case where malloc fails in first_modevent. > >> first_modevent will return ENOMEM, but the module will remain loaded. > >> Now when the second module goes and loads, it calls into the first > >> module, which is not initialized properly, and promptly crashes when > >> test_function() dereferences a null pointer. > >> > >> It seems to me that a module should be unloaded if it returns an error > >> from its MOD_LOAD handler. However, that's easier said than done. > >> The MOD_LOAD handler is called from a SYSINIT, and there's no > >> immediately obvious way to pass information about the failure from the > >> SYSINIT to the kernel linker. Anybody have any thoughts on this? > > > > Yeah, it's not easy to fix. Probably we could patch the kernel linker to > > notice if any of the modules for a given linker file had errors during > > initialization and trigger an unload if that occurs. I don't think this would > > be too hard to do. > > John, > > please note that for this testcase we would need to prevent second module's > modevent from being executed at all. > Perhaps a module shouldn't be considered as loaded until modevent caller marks it > as successfully initialized, but I haven't looked at the actual code. Well, if these two event handlers are in the same module, I think that is a bug in the module really. I tend to collapse such things down to a single event handler per kld just so I can really get the ordering correct anyway. :) If they are in two separate .ko files then the other solution would work. We could also hack the module code to mark a linker_file as 'broken' and have the module_helper sysinit not call mod_load if the containing file is 'broken', etc. -- John Baldwin
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?201008231110.04552.jhb>