Date: Fri, 20 Aug 2010 14:26:19 -0700 From: Garrett Cooper <gcooper@FreeBSD.org> To: Ryan Stone <rysto32@gmail.com> Cc: freebsd-hackers@freebsd.org Subject: Re: kld modules remain loaded if MOD_LOAD handler returns an error Message-ID: <AANLkTik-QzKV63AViZ34v_-o0WkJcLPEMwg0m3h%2B7y3Y@mail.gmail.com> In-Reply-To: <AANLkTi=qJ3WZChWg5axnuH8OBMxzd6JrnmfsmXHqPY2Q@mail.gmail.com> References: <AANLkTi=qJ3WZChWg5axnuH8OBMxzd6JrnmfsmXHqPY2Q@mail.gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Fri, Aug 20, 2010 at 10:13 AM, Ryan Stone <rysto32@gmail.com> wrote:
> Consider the following modules:
>
> /* first.c */
> static int *test;
>
> int
> test_function(void)
> {
> =A0 =A0return *test;
> }
>
> static int
> first_modevent(struct module *m, int what, void *arg)
> {
> =A0 =A0 =A0 =A0int err =3D 0;
>
> =A0 =A0 =A0 =A0switch (what) {
> =A0 =A0 =A0 =A0case MOD_LOAD: =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* kldload *=
/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0test =3D malloc(sizeof(int), M_TEMP, M_NOW=
AIT | M_ZERO);
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0if (!test)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err =3D ENOMEM;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0case MOD_UNLOAD: =A0 =A0 =A0 =A0 =A0 =A0 =A0/* kldunload *=
/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0default:
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err =3D EINVAL;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0return(err);
> }
>
> static moduledata_t first_mod =3D {
> =A0 =A0 =A0 =A0"first",
> =A0 =A0 =A0 =A0first_modevent,
> =A0 =A0 =A0 =A0NULL
> };
>
> 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)
> {
> =A0 =A0 =A0 =A0int err =3D 0;
>
> =A0 =A0 =A0 =A0switch (what) {
> =A0 =A0 =A0 =A0case MOD_LOAD: =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0/* kldload *=
/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0test_function();
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0case MOD_UNLOAD: =A0 =A0 =A0 =A0 =A0 =A0 =A0/* kldunload *=
/
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0default:
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0err =3D EINVAL;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0break;
> =A0 =A0 =A0 =A0}
> =A0 =A0 =A0 =A0return(err);
> }
>
> static moduledata_t second_mod =3D {
> =A0 =A0 =A0 =A0"second",
> =A0 =A0 =A0 =A0second_modevent,
> =A0 =A0 =A0 =A0NULL
> };
>
> 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. =A0However, 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. =A0Anybody 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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTik-QzKV63AViZ34v_-o0WkJcLPEMwg0m3h%2B7y3Y>