From owner-freebsd-hackers@FreeBSD.ORG Fri Aug 20 21:26:28 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 C53411065693 for ; Fri, 20 Aug 2010 21:26:28 +0000 (UTC) (envelope-from yanegomi@gmail.com) Received: from mail-bw0-f54.google.com (mail-bw0-f54.google.com [209.85.214.54]) by mx1.freebsd.org (Postfix) with ESMTP id C59498FC18 for ; Fri, 20 Aug 2010 21:26:19 +0000 (UTC) Received: by mail-bw0-f54.google.com with SMTP id 20so25091bwz.13 for ; Fri, 20 Aug 2010 14:26:19 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=gamma; h=domainkey-signature:mime-version:received:sender:received :in-reply-to:references:date:x-google-sender-auth:message-id:subject :from:to:cc:content-type:content-transfer-encoding; bh=20iIZvEe/U2lriN7wCE+/dEmnOWU3H4tte3ThnM97Ik=; b=DvXbcnjeqF8315KG/Fkii4+wMviW5gw4IKwUDm5SpH/jP5w+v+gax/LGtGXFnMAbvW SMXYY6k1x1pIeUBF+RL2opb48OnROId9s6ZGtbQt5zcLMMM3Xwm6Vhy4exbxgL6QoBpG 5hP7DwZ7u/inbUb/IBrl3+5APoAC3qUj+VSZg= DomainKey-Signature: a=rsa-sha1; c=nofws; d=gmail.com; s=gamma; h=mime-version:sender:in-reply-to:references:date :x-google-sender-auth:message-id:subject:from:to:cc:content-type :content-transfer-encoding; b=kPvZfsgi2l7DU5CQdKoYOBZrCdIdCxjJnjyekj+e7TuOH+NAgP33ftu0Mi07isc3ct NWlc358tEJpAMcv2IAW8OOOIJNZDFqW64yF5BJcxXdMsfzBz26Xs/CB1om9FJgu0a9al 9IwTJXM6GVhp6zGYkV2yQfjN7jR+/Z/d5q4hg= MIME-Version: 1.0 Received: by 10.204.68.136 with SMTP id v8mr1372248bki.88.1282339579405; Fri, 20 Aug 2010 14:26:19 -0700 (PDT) Sender: yanegomi@gmail.com Received: by 10.204.82.6 with HTTP; Fri, 20 Aug 2010 14:26:19 -0700 (PDT) In-Reply-To: References: Date: Fri, 20 Aug 2010 14:26:19 -0700 X-Google-Sender-Auth: YndQrd0bWcj9s5XgccCG1NYZNiI Message-ID: From: Garrett Cooper To: Ryan Stone Content-Type: text/plain; charset=ISO-8859-1 Content-Transfer-Encoding: quoted-printable Cc: freebsd-hackers@freebsd.org 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: Fri, 20 Aug 2010 21:26:28 -0000 On Fri, Aug 20, 2010 at 10:13 AM, Ryan Stone 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