From owner-freebsd-hackers@FreeBSD.ORG Mon Aug 23 12:40:29 2010 Return-Path: Delivered-To: freebsd-hackers@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 788351065696 for ; Mon, 23 Aug 2010 12:40:29 +0000 (UTC) (envelope-from jhb@freebsd.org) Received: from cyrus.watson.org (cyrus.watson.org [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 47AF48FC16 for ; Mon, 23 Aug 2010 12:40:29 +0000 (UTC) Received: from bigwig.baldwin.cx (66.111.2.69.static.nyinternet.net [66.111.2.69]) by cyrus.watson.org (Postfix) with ESMTPSA id E3BBD46B37; Mon, 23 Aug 2010 08:40:28 -0400 (EDT) Received: from jhbbsd.localnet (smtp.hudson-trading.com [209.249.190.9]) by bigwig.baldwin.cx (Postfix) with ESMTPSA id D55BD8A04F; Mon, 23 Aug 2010 08:40:27 -0400 (EDT) From: John Baldwin To: freebsd-hackers@freebsd.org Date: Mon, 23 Aug 2010 08:10:04 -0400 User-Agent: KMail/1.13.5 (FreeBSD/7.3-CBSD-20100819; KDE/4.4.5; amd64; ; ) References: In-Reply-To: MIME-Version: 1.0 Content-Type: Text/Plain; charset="iso-8859-1" Content-Transfer-Encoding: 7bit Message-Id: <201008230810.04710.jhb@freebsd.org> X-Greylist: Sender succeeded SMTP AUTH, not delayed by milter-greylist-4.0.1 (bigwig.baldwin.cx); Mon, 23 Aug 2010 08:40:28 -0400 (EDT) X-Virus-Scanned: clamav-milter 0.95.1 at bigwig.baldwin.cx X-Virus-Status: Clean X-Spam-Status: No, score=-2.6 required=4.2 tests=AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on bigwig.baldwin.cx Cc: Ryan Stone Subject: Re: kld modules remain loaded if MOD_LOAD handler returns an error X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 23 Aug 2010 12:40:29 -0000 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