Skip site navigation (1)Skip section navigation (2)
Date:      Fri, 27 Aug 2004 21:45:44 +0200
From:      Andre Oppermann <andre@freebsd.org>
To:        Darren Reed <darrenr@hub.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 ...
Message-ID:  <412F8F68.92BDEEB0@freebsd.org>
References:  <200408271516.i7RFGO8L061926@repoman.freebsd.org> <200408271812.18748.max@love2party.net> <20040827192307.GA55748@hub.freebsd.org>

next in thread | previous in thread | raw e-mail | index | archive | help
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



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?412F8F68.92BDEEB0>