From owner-cvs-src@FreeBSD.ORG Tue Oct 19 22:35:43 2004 Return-Path: Delivered-To: cvs-src@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 97A6216A4CE; Tue, 19 Oct 2004 22:35:43 +0000 (GMT) Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.173]) by mx1.FreeBSD.org (Postfix) with ESMTP id 170DF43D48; Tue, 19 Oct 2004 22:35:43 +0000 (GMT) (envelope-from max@love2party.net) Received: from [212.227.126.206] (helo=mrelayng.kundenserver.de) by moutng.kundenserver.de with esmtp (Exim 3.35 #1) id 1CK2Zy-00060C-00; Wed, 20 Oct 2004 00:35:42 +0200 Received: from [217.227.158.113] (helo=donor.laier.local) by mrelayng.kundenserver.de with asmtp (TLSv1:RC4-MD5:128) (Exim 3.35 #1) id 1CK2Zw-0005yE-00; Wed, 20 Oct 2004 00:35:41 +0200 From: Max Laier To: Andre Oppermann Date: Wed, 20 Oct 2004 00:35:10 +0200 User-Agent: KMail/1.7 References: <200410191513.i9JFDUbf072176@repoman.freebsd.org> <200410192329.46723.max@love2party.net> <41758B35.D5340AEA@freebsd.org> In-Reply-To: <41758B35.D5340AEA@freebsd.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart3269231.sYBdOc9t0i"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <200410200035.19752.max@love2party.net> X-Provags-ID: kundenserver.de abuse@kundenserver.de auth:61c499deaeeba3ba5be80f48ecc83056 cc: Sam Leffler cc: src-committers@freebsd.org cc: cvs-all@freebsd.org cc: cvs-src@freebsd.org Subject: Re: cvs commit: src/sys/sys protosw.h src/sys/kern uipc_domain.cuipc_socket2.c X-BeenThere: cvs-src@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: CVS commit messages for the src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 19 Oct 2004 22:35:43 -0000 --nextPart3269231.sYBdOc9t0i Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Tuesday 19 October 2004 23:46, Andre Oppermann wrote: <...> > > problems. For example, in ip_icmp.c line 457 ff we have: > > > > ctlfunc =3D > > inetsw[ip_protox[icp->icmp_ip.ip_p]].pr_ctlinput; if (ctlfunc) > > (*ctlfunc)(code, (struct sockaddr *)&icmpsrc, > > (void *)&icp->icmp_ip); > > Ok, this one is easy to fix. I'll audit the code for any other of these > abuses. One of many, I am afraid. > > This is clearly a problem if we can remove protocols. There might be mo= re > > places where we (temporary) cache values from the protocol array. Anoth= er > > problem might be that we check for protocol existence early and assume > > that this remains true ... > > Well, too bad if some code tries to remember this. Doesn't hurt then. > From my reading of many parts of the netinet/* code this is usually not > a problem and the code is rather well behaved. I refuse to take this > argument as reason to not have loadable protocols. "... usually ... rather ..." I really urge you, to reconsider. Many have=20 argumented in the same way. I understand that it is nice to have this=20 possibility, but it *does* cause *real* problems! > > I'd suggest, that you remove the possibility to remove protocols > > completely. It is very likely that there are no races with adding > > protocols - though it might take "some time" for the protocol to be ful= ly > > useable - but the removal is critical. > > I don't think it should be a one-way street. To be able to unload > protocols is an important but seldomly used function and it's certainly n= ot > that a crash is guarnteed. Far from it. > > > We also have to check that really all code can cope with the addition a= nd > > properly reinitializes it's view of the protocol arrays. > > The point of the protocol arrays is precisely to have them as the only > and sole place where such information is stored. Any code that copies > any part of it to its own private structures is horribly broken by design > and must be fixed anyway! (BTW: I'm not aware of any code within netinet= /* > that does this.) I mentioned one above, I am sure there are others. Some as obvious as the o= ne=20 above, some less so ... > > Another point: If you really want to keep the possibility to remove a > > protocol, you have to introduce some busy counter that pervents removal > > while the kernel is inside a protocol function. This has to be handled = by > > the protocol itself, but it has to be taken care of somehow. > > Yes, the protocol has to be able to handle its own unloading. I have > documented that fact. If a protocol in unable to do so it should simply > refuse any unload attempts with EBUSY. Divert can be paniced with the sysctl code, btw. You have something like: lock; unlock; SYSCTL_OUT; <-- this can be made to take *some* time lock; <-- this will panic once the lock is destroyed And there are other problems. Yes, it is not a problem in the common case, = but=20 you have to account for edge cases as well! =2D-=20 /"\ Best regards, | mlaier@freebsd.org \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | mlaier@EFnet / \ ASCII Ribbon Campaign | Against HTML Mail and News --nextPart3269231.sYBdOc9t0i Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (FreeBSD) iD8DBQBBdZanXyyEoT62BG0RAqacAJ4p7xH50oz47gf+QjkMVZd9FeSvgACfUj4e bC0+2SLN9ZjnBlbH+eoX/S0= =mwLt -----END PGP SIGNATURE----- --nextPart3269231.sYBdOc9t0i--