From owner-cvs-src@FreeBSD.ORG Fri Aug 27 19:45:42 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 BC43F16A4CE for ; Fri, 27 Aug 2004 19:45:42 +0000 (GMT) Received: from c00l3r.networx.ch (c00l3r.networx.ch [62.48.2.2]) by mx1.FreeBSD.org (Postfix) with ESMTP id F007443D67 for ; Fri, 27 Aug 2004 19:45:41 +0000 (GMT) (envelope-from andre@freebsd.org) Received: (qmail 59842 invoked from network); 27 Aug 2004 19:44:17 -0000 Received: from unknown (HELO freebsd.org) ([62.48.0.53]) (envelope-sender ) by c00l3r.networx.ch (qmail-ldap-1.03) with SMTP for ; 27 Aug 2004 19:44:17 -0000 Message-ID: <412F8F68.92BDEEB0@freebsd.org> Date: Fri, 27 Aug 2004 21:45:44 +0200 From: Andre Oppermann X-Mailer: Mozilla 4.8 [en] (Windows NT 5.0; U) X-Accept-Language: en MIME-Version: 1.0 To: Darren Reed References: <200408271516.i7RFGO8L061926@repoman.freebsd.org> <200408271812.18748.max@love2party.net> <20040827192307.GA55748@hub.freebsd.org> Content-Type: text/plain; charset=us-ascii Content-Transfer-Encoding: 7bit cc: Max Laier cc: src-committers@FreeBSD.org cc: cvs-all@FreeBSD.org cc: cvs-src@FreeBSD.org Subject: Re: cvs commit: src/share/man/man4 ipfirewall.4 src/share/man/man9 pfil.9 src/sys/alpha/conf GENERIC src/sys/amd64/conf GENERIC src/sys/conf NOTES files options src/sys/i386/conf GENERIC src/sys/ia64/conf GENERIC SKI src/sys/modules/bridge Makefile ... 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: Fri, 27 Aug 2004 19:45:42 -0000 Darren Reed wrote: > > On Fri, Aug 27, 2004 at 06:12:11PM +0200, Max Laier wrote: > > Maybe we should hide: > > if (inet_pfil_hook.ph_busy_count == -1) > > There's now a double check on ph_busy_count for inet & inet6 as it's > first statement in pfil_run_hooks(). Seems a bit silly... It's not. There is quite a bit of code that follows the pfil_run_hooks() to look for certain things that might have happend to a packet. If no hooks are connected we can save the work and simply jump over it. Take a look the code that is jumped over. > > behind a macro in case we modify the locking for pfil_hooks in the future. I > > am thinking of something like: > > if (PFIL_IS_EMPTY(&inet_pfil_hook)) > > Unless pfil(9) is being used with a protocol that has been loaded via > an LKM (and can therefore disappear), there should be no risk here to > warrant the use of locking. Locking is used to protect changes to the hooks. If you hook/unhook there might be another CPU traversing the hooks while you yank them underneath it. Panic. > The pfil locking should be overhauled, however, with the main aim > to replace PFIL_*LOCK() with the use of sx(9). Please read the reply from Max. He already explained why sx(9) is inappropriate. > As an example of the evilness of this, if you've got (say) ipfw loaded > and you want to enable ipf or pf, there's a security hole that could > allow a packet to flow through the system without any of them kicking > in (ph_want_write.) I agree that this is not perfect. Max already said he wants to revisit pfil_hooks locking in the future. -- Andre