Date: Mon, 23 Aug 2010 18:04:20 +0300 From: Andriy Gapon <avg@icyb.net.ua> To: John Baldwin <jhb@freebsd.org> Cc: freebsd-hackers@freebsd.org, Ryan Stone <rysto32@gmail.com> Subject: Re: kld modules remain loaded if MOD_LOAD handler returns an error Message-ID: <4C728DF4.2060203@icyb.net.ua> In-Reply-To: <201008230810.04710.jhb@freebsd.org> References: <AANLkTi=qJ3WZChWg5axnuH8OBMxzd6JrnmfsmXHqPY2Q@mail.gmail.com> <201008230810.04710.jhb@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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.
--
Andriy Gapon
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?4C728DF4.2060203>