kld modules remain loaded if MOD_LOAD handler returns an error

[John Baldwin]

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 Baldwin