kld modules remain loaded if MOD_LOAD handler returns an error

[Garrett Cooper]

On Fri, Aug 20, 2010 at 10:13 AM, Ryan Stone <rysto32 at gmail.com> 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?

I saw similar issues as well with another driver on 6.3 and 7.1.
Looking over kern_kldload and kldload in sys/kern/kern_linker.c, it
doesn't appear that there's an issue with error catching, but I didn't
attempt to trace down the call stack further than that.
-Garrett