2010/8/23 Andriy Gapon <a...@icyb.net.ua>: > 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.
I replied in private, but I really agree with Andriy here. Thanks, Attilio -- Peace can only be achieved by understanding - A. Einstein _______________________________________________ freebsd-hackers@freebsd.org mailing list http://lists.freebsd.org/mailman/listinfo/freebsd-hackers To unsubscribe, send any mail to "freebsd-hackers-unsubscr...@freebsd.org"