Date: Fri, 20 Aug 2010 13:13:53 -0400 From: Ryan Stone <rysto32@gmail.com> To: freebsd-hackers@freebsd.org Subject: kld modules remain loaded if MOD_LOAD handler returns an error Message-ID: <AANLkTi=qJ3WZChWg5axnuH8OBMxzd6JrnmfsmXHqPY2Q@mail.gmail.com>
next in thread | raw e-mail | index | archive | help
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?
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTi=qJ3WZChWg5axnuH8OBMxzd6JrnmfsmXHqPY2Q>