From owner-freebsd-pf@FreeBSD.ORG Mon Dec 19 12:19:30 2005 Return-Path: X-Original-To: freebsd-pf@freebsd.org Delivered-To: freebsd-pf@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 47B6616A41F; Mon, 19 Dec 2005 12:19:30 +0000 (GMT) (envelope-from max@love2party.net) Received: from moutng.kundenserver.de (moutng.kundenserver.de [212.227.126.177]) by mx1.FreeBSD.org (Postfix) with ESMTP id 27E8A43D53; Mon, 19 Dec 2005 12:19:28 +0000 (GMT) (envelope-from max@love2party.net) Received: from [84.163.246.200] (helo=amd64.laiers.local) by mrelayeu.kundenserver.de (node=mrelayeu2) with ESMTP (Nemesis), id 0MKwtQ-1EoJzD1dnc-0000mq; Mon, 19 Dec 2005 13:19:28 +0100 From: Max Laier Organization: FreeBSD To: freebsd-pf@freebsd.org Date: Mon, 19 Dec 2005 13:19:29 +0100 User-Agent: KMail/1.8.3 References: <200507201858.j6KIwRNZ097685@repoman.freebsd.org> <20051218090822.GA8358@heff.fud.org.nz> <20051219032129.GA10219@heff.fud.org.nz> In-Reply-To: <20051219032129.GA10219@heff.fud.org.nz> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="nextPart1186789.5yYGLEeU4N"; protocol="application/pgp-signature"; micalg=pgp-sha1 Content-Transfer-Encoding: 7bit Message-Id: <200512191319.35764.max@love2party.net> X-Provags-ID: kundenserver.de abuse@kundenserver.de login:61c499deaeeba3ba5be80f48ecc83056 Cc: Andrew Thompson , dhartmei@freebsd.org Subject: Re: cvs commit: src/sys/contrib/pf/net pf.c pfvar.h X-BeenThere: freebsd-pf@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "Technical discussion and general questions about packet filter \(pf\)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 19 Dec 2005 12:19:30 -0000 --nextPart1186789.5yYGLEeU4N Content-Type: multipart/mixed; boundary="Boundary-01=_SVqpDalcTOfvl3U" Content-Transfer-Encoding: 7bit Content-Disposition: inline --Boundary-01=_SVqpDalcTOfvl3U Content-Type: text/plain; charset="iso-8859-1" Content-Transfer-Encoding: quoted-printable Content-Disposition: inline On Monday 19 December 2005 04:21, Andrew Thompson wrote: > On Sun, Dec 18, 2005 at 10:08:22PM +1300, Andrew Thompson wrote: > > On Wed, Jul 20, 2005 at 06:58:27PM +0000, Max Laier wrote: > > > mlaier 2005-07-20 18:58:27 UTC > > > > > > FreeBSD src repository > > > > > > Modified files: > > > sys/contrib/pf/net pf.c pfvar.h > > > Log: > > > Prevent a race condition. As pf_send_tcp() - called for expired > > > synproxy states - has to drop the lock when calling back to > > > ip_output(), the state purge timeout might run and gc the state. This > > > results in a rb-tree inconsistency. With this change we flag expiring > > > states while holding the lock and back off if the flag is already set. > > > > This commit seems to have broken net/pfflowd in ports. It still recieves > > packets from pfsync0 but nothing with action =3D=3D PFSYNC_ACT_DEL. > > More specifically the pfsync_delete_state() macro is broken. > > pf_purge_expired_state(struct pf_state *cur) > { > if (cur->sync_flags & PFSTATE_EXPIRING) > return; > cur->sync_flags |=3D PFSTATE_EXPIRING; > <...> > pfsync_delete_state(cur); > > > But this will not do anything since sync_flags is not non-zero, as it is > checked in the macro. > > #define pfsync_delete_state(st) do { \ > if (!st->sync_flags) \ > pfsync_pack_state(PFSYNC_ACT_DEL, (st), \ > PFSYNC_FLAG_COMPRESS); \ > st->sync_flags &=3D ~PFSTATE_FROMSYNC; \ > } while (0) Autsch - good catch! Quick fix, using pad-space, attached. Not sure if th= is=20 is the best sollution, but it certainly is the least invasive. Looking at= =20 the current OpenBSD code we still have 8bit padding somewhere in struct=20 pf_state, so we can continue like this. I will commit and MFC this, unless= I=20 hear complains. =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 --Boundary-01=_SVqpDalcTOfvl3U Content-Type: text/x-diff; charset="iso-8859-1"; name="pf_state_expiring.diff" Content-Transfer-Encoding: quoted-printable Content-Disposition: attachment; filename="pf_state_expiring.diff" Index: pf.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pf.c,v retrieving revision 1.38 diff -u -r1.38 pf.c =2D-- pf.c 5 Dec 2005 11:58:31 -0000 1.38 +++ pf.c 19 Dec 2005 12:11:22 -0000 @@ -1102,9 +1102,9 @@ pf_purge_expired_state(struct pf_state *cur) { #ifdef __FreeBSD__ =2D if (cur->sync_flags & PFSTATE_EXPIRING) + if (cur->local_flags & PFSTATE_EXPIRING) return; =2D cur->sync_flags |=3D PFSTATE_EXPIRING; + cur->local_flags |=3D PFSTATE_EXPIRING; #endif if (cur->src.state =3D=3D PF_TCPS_PROXY_DST) pf_send_tcp(cur->rule.ptr, cur->af, Index: pfvar.h =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pfvar.h,v retrieving revision 1.12 diff -u -r1.12 pfvar.h =2D-- pfvar.h 20 Jul 2005 18:58:27 -0000 1.12 +++ pfvar.h 19 Dec 2005 12:09:51 -0000 @@ -791,9 +791,11 @@ #define PFSTATE_FROMSYNC 0x02 #define PFSTATE_STALE 0x04 #ifdef __FreeBSD__ =2D#define PFSTATE_EXPIRING 0x10 =2D#endif + u_int8_t local_flags; +#define PFSTATE_EXPIRING 0x01 +#else u_int8_t pad; +#endif }; =20 TAILQ_HEAD(pf_rulequeue, pf_rule); --Boundary-01=_SVqpDalcTOfvl3U-- --nextPart1186789.5yYGLEeU4N Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.2 (FreeBSD) iD8DBQBDpqVXXyyEoT62BG0RAjLPAJ92Sy/WOyc2nXS50gs75dS8nEsAYgCdENKW YYlDMlnRo3woordK9q6pC4c= =/lIX -----END PGP SIGNATURE----- --nextPart1186789.5yYGLEeU4N--