Skip site navigation (1)Skip section navigation (2)
Date:      Wed, 19 Aug 2009 00:10:11 +0000 (UTC)
From:      Max Laier <mlaier@FreeBSD.org>
To:        src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org
Subject:   svn commit: r196372 - head/sys/contrib/pf/net
Message-ID:  <200908190010.n7J0ABEk032189@svn.freebsd.org>

next in thread | raw e-mail | index | archive | help
Author: mlaier
Date: Wed Aug 19 00:10:10 2009
New Revision: 196372
URL: http://svn.freebsd.org/changeset/base/196372

Log:
  If we cannot immediately get the pf_consistency_lock in the purge thread,
  restart the scan after acquiring the lock the hard way.  Otherwise we might
  end up with a dead reference.
  
  Reported by:		pfsense
  Reviewed by:		eri
  Initial patch by:	eri
  Tested by:		pfsense
  Approved by:		re (kib)

Modified:
  head/sys/contrib/pf/net/pf.c
  head/sys/contrib/pf/net/pfvar.h

Modified: head/sys/contrib/pf/net/pf.c
==============================================================================
--- head/sys/contrib/pf/net/pf.c	Tue Aug 18 21:13:00 2009	(r196371)
+++ head/sys/contrib/pf/net/pf.c	Wed Aug 19 00:10:10 2009	(r196372)
@@ -971,6 +971,9 @@ void
 pf_purge_thread(void *v)
 {
 	int nloops = 0, s;
+#ifdef __FreeBSD__
+	int locked;
+#endif
 
 	for (;;) {
 		tsleep(pf_purge_thread, PWAIT, "pftm", 1 * hz);
@@ -978,14 +981,19 @@ pf_purge_thread(void *v)
 #ifdef __FreeBSD__
 		sx_slock(&pf_consistency_lock);
 		PF_LOCK();
+		locked = 0;
 
 		if (pf_end_threads) {
-			pf_purge_expired_states(pf_status.states);
+			PF_UNLOCK();
+			sx_sunlock(&pf_consistency_lock);
+			sx_xlock(&pf_consistency_lock);
+			PF_LOCK();
+			pf_purge_expired_states(pf_status.states, 1);
 			pf_purge_expired_fragments();
-			pf_purge_expired_src_nodes(0);
+			pf_purge_expired_src_nodes(1);
 			pf_end_threads++;
 
-			sx_sunlock(&pf_consistency_lock);
+			sx_xunlock(&pf_consistency_lock);
 			PF_UNLOCK();
 			wakeup(pf_purge_thread);
 			kproc_exit(0);
@@ -994,20 +1002,44 @@ pf_purge_thread(void *v)
 		s = splsoftnet();
 
 		/* process a fraction of the state table every second */
+#ifdef __FreeBSD__
+		if(!pf_purge_expired_states(1 + (pf_status.states
+		    / pf_default_rule.timeout[PFTM_INTERVAL]), 0)) {
+			PF_UNLOCK();
+			sx_sunlock(&pf_consistency_lock);
+			sx_xlock(&pf_consistency_lock);
+			PF_LOCK();
+			locked = 1;
+
+			pf_purge_expired_states(1 + (pf_status.states
+			    / pf_default_rule.timeout[PFTM_INTERVAL]), 1);
+		}
+#else
 		pf_purge_expired_states(1 + (pf_status.states
 		    / pf_default_rule.timeout[PFTM_INTERVAL]));
+#endif
 
 		/* purge other expired types every PFTM_INTERVAL seconds */
 		if (++nloops >= pf_default_rule.timeout[PFTM_INTERVAL]) {
 			pf_purge_expired_fragments();
-			pf_purge_expired_src_nodes(0);
+			if (!pf_purge_expired_src_nodes(locked)) {
+				PF_UNLOCK();
+				sx_sunlock(&pf_consistency_lock);
+				sx_xlock(&pf_consistency_lock);
+				PF_LOCK();
+				locked = 1;
+				pf_purge_expired_src_nodes(1);
+			}
 			nloops = 0;
 		}
 
 		splx(s);
 #ifdef __FreeBSD__
 		PF_UNLOCK();
-		sx_sunlock(&pf_consistency_lock);
+		if (locked)
+			sx_xunlock(&pf_consistency_lock);
+		else
+			sx_sunlock(&pf_consistency_lock);
 #endif
 	}
 }
@@ -1056,8 +1088,13 @@ pf_state_expires(const struct pf_state *
 	return (state->expire + timeout);
 }
 
+#ifdef __FreeBSD__
+int
+pf_purge_expired_src_nodes(int waslocked)
+#else
 void
 pf_purge_expired_src_nodes(int waslocked)
+#endif
 {
 	 struct pf_src_node		*cur, *next;
 	 int				 locked = waslocked;
@@ -1068,12 +1105,8 @@ pf_purge_expired_src_nodes(int waslocked
 		 if (cur->states <= 0 && cur->expire <= time_second) {
 			 if (! locked) {
 #ifdef __FreeBSD__
-				 if (!sx_try_upgrade(&pf_consistency_lock)) {
-					 PF_UNLOCK();
-					 sx_sunlock(&pf_consistency_lock);
-					 sx_xlock(&pf_consistency_lock);
-					 PF_LOCK();
-				 }
+				 if (!sx_try_upgrade(&pf_consistency_lock))
+				 	return (0);
 #else
 				 rw_enter_write(&pf_consistency_lock);
 #endif
@@ -1100,6 +1133,10 @@ pf_purge_expired_src_nodes(int waslocked
 #else
 		rw_exit_write(&pf_consistency_lock);
 #endif
+
+#ifdef __FreeBSD__
+	return (1);
+#endif
 }
 
 void
@@ -1202,12 +1239,21 @@ pf_free_state(struct pf_state *cur)
 	pf_status.states--;
 }
 
+#ifdef __FreeBSD__
+int
+pf_purge_expired_states(u_int32_t maxcheck, int waslocked)
+#else
 void
 pf_purge_expired_states(u_int32_t maxcheck)
+#endif
 {
 	static struct pf_state	*cur = NULL;
 	struct pf_state		*next;
+#ifdef __FreeBSD__
+	int 			 locked = waslocked;
+#else
 	int 			 locked = 0;
+#endif
 
 	while (maxcheck--) {
 		/* wrap to start of list when we hit the end */
@@ -1224,12 +1270,8 @@ pf_purge_expired_states(u_int32_t maxche
 			/* free unlinked state */
 			if (! locked) {
 #ifdef __FreeBSD__
-				 if (!sx_try_upgrade(&pf_consistency_lock)) {
-					 PF_UNLOCK();
-					 sx_sunlock(&pf_consistency_lock);
-					 sx_xlock(&pf_consistency_lock);
-					 PF_LOCK();
-				 }
+				 if (!sx_try_upgrade(&pf_consistency_lock))
+				 	return (0);
 #else
 				rw_enter_write(&pf_consistency_lock);
 #endif
@@ -1241,12 +1283,8 @@ pf_purge_expired_states(u_int32_t maxche
 			pf_unlink_state(cur);
 			if (! locked) {
 #ifdef __FreeBSD__
-				 if (!sx_try_upgrade(&pf_consistency_lock)) {
-					 PF_UNLOCK();
-					 sx_sunlock(&pf_consistency_lock);
-					 sx_xlock(&pf_consistency_lock);
-					 PF_LOCK();
-				 }
+				 if (!sx_try_upgrade(&pf_consistency_lock))
+				 	return (0);
 #else
 				rw_enter_write(&pf_consistency_lock);
 #endif
@@ -1257,10 +1295,13 @@ pf_purge_expired_states(u_int32_t maxche
 		cur = next;
 	}
 
-	if (locked)
 #ifdef __FreeBSD__
+	if (!waslocked && locked)
 		sx_downgrade(&pf_consistency_lock);
+
+	return (1);
 #else
+	if (locked)
 		rw_exit_write(&pf_consistency_lock);
 #endif
 }

Modified: head/sys/contrib/pf/net/pfvar.h
==============================================================================
--- head/sys/contrib/pf/net/pfvar.h	Tue Aug 18 21:13:00 2009	(r196371)
+++ head/sys/contrib/pf/net/pfvar.h	Wed Aug 19 00:10:10 2009	(r196372)
@@ -1593,8 +1593,13 @@ extern struct pool		 pf_state_pl, pf_alt
 extern struct pool		 pf_state_scrub_pl;
 #endif
 extern void			 pf_purge_thread(void *);
+#ifdef __FreeBSD__
+extern int			 pf_purge_expired_src_nodes(int);
+extern int			 pf_purge_expired_states(u_int32_t, int);
+#else
 extern void			 pf_purge_expired_src_nodes(int);
 extern void			 pf_purge_expired_states(u_int32_t);
+#endif
 extern void			 pf_unlink_state(struct pf_state *);
 extern void			 pf_free_state(struct pf_state *);
 extern int			 pf_insert_state(struct pfi_kif *,



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908190010.n7J0ABEk032189>