Date: Mon, 19 Dec 2005 13:19:29 +0100 From: Max Laier <max@love2party.net> To: freebsd-pf@freebsd.org Cc: Andrew Thompson <thompsa@freebsd.org>, dhartmei@freebsd.org Subject: Re: cvs commit: src/sys/contrib/pf/net pf.c pfvar.h Message-ID: <200512191319.35764.max@love2party.net> In-Reply-To: <20051219032129.GA10219@heff.fud.org.nz> References: <200507201858.j6KIwRNZ097685@repoman.freebsd.org> <20051218090822.GA8358@heff.fud.org.nz> <20051219032129.GA10219@heff.fud.org.nz>
next in thread | previous in thread | raw e-mail | index | archive | help
--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--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200512191319.35764.max>