From owner-cvs-all@FreeBSD.ORG  Fri Aug 27 19:23:07 2004
Return-Path: <owner-cvs-all@FreeBSD.ORG>
Delivered-To: cvs-all@freebsd.org
Received: by hub.freebsd.org (Postfix, from userid 680)
	id 80C0B16A4D3; Fri, 27 Aug 2004 19:23:07 +0000 (GMT)
Date: Fri, 27 Aug 2004 19:23:07 +0000
From: Darren Reed <darrenr@hub.freebsd.org>
To: Max Laier <max@love2party.net>
Message-ID: <20040827192307.GA55748@hub.freebsd.org>
References: <200408271516.i7RFGO8L061926@repoman.freebsd.org>
	<200408271812.18748.max@love2party.net>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <200408271812.18748.max@love2party.net>
User-Agent: Mutt/1.4.1i
cc: cvs-src@FreeBSD.org
cc: src-committers@FreeBSD.org
cc: Andre Oppermann <andre@FreeBSD.org>
cc: cvs-all@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-all@freebsd.org
X-Mailman-Version: 2.1.1
Precedence: list
List-Id: CVS commit messages for the entire tree <cvs-all.freebsd.org>
List-Unsubscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-all>,
	<mailto:cvs-all-request@freebsd.org?subject=unsubscribe>
List-Archive: <http://lists.freebsd.org/pipermail/cvs-all>
List-Post: <mailto:cvs-all@freebsd.org>
List-Help: <mailto:cvs-all-request@freebsd.org?subject=help>
List-Subscribe: <http://lists.freebsd.org/mailman/listinfo/cvs-all>,
	<mailto:cvs-all-request@freebsd.org?subject=subscribe>
X-List-Received-Date: Fri, 27 Aug 2004 19:23:07 -0000

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...

> 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.

The pfil locking should be overhauled, however, with the main aim
to replace PFIL_*LOCK() with the use of sx(9).

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.)

Darren