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 == 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 |= 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 &= ~PFSTATE_FROMSYNC; \
> } while (0)Autsch - good catch! Quick fix, using pad-space, attached. Not sure if this is the best sollution, but it certainly is the least invasive. Looking at the current OpenBSD code we still have 8bit padding somewhere in struct pf_state, so we can continue like this. I will commit and MFC this, unless I hear complains. -- /"\ Best regards, | [EMAIL PROTECTED] \ / Max Laier | ICQ #67774661 X http://pf4freebsd.love2party.net/ | [EMAIL PROTECTED] / \ ASCII Ribbon Campaign | Against HTML Mail and News
Index: pf.c
===================================================================
RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pf.c,v
retrieving revision 1.38
diff -u -r1.38 pf.c
--- 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__
- if (cur->sync_flags & PFSTATE_EXPIRING)
+ if (cur->local_flags & PFSTATE_EXPIRING)
return;
- cur->sync_flags |= PFSTATE_EXPIRING;
+ cur->local_flags |= PFSTATE_EXPIRING;
#endif
if (cur->src.state == PF_TCPS_PROXY_DST)
pf_send_tcp(cur->rule.ptr, cur->af,
Index: pfvar.h
===================================================================
RCS file: /usr/store/mlaier/fcvs/src/sys/contrib/pf/net/pfvar.h,v
retrieving revision 1.12
diff -u -r1.12 pfvar.h
--- 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__
-#define PFSTATE_EXPIRING 0x10
-#endif
+ u_int8_t local_flags;
+#define PFSTATE_EXPIRING 0x01
+#else
u_int8_t pad;
+#endif
};
TAILQ_HEAD(pf_rulequeue, pf_rule);
pgp2t2WpozKSz.pgp
Description: PGP signature
