On Wednesday 12 August 2009 21:16:09 Peter Jeremy wrote:
> My firewall (7.2p3/i386) recently panic'd:
> Fatal trap 12: page fault while in kernel mode
> fault virtual address   = 0x1065e
> fault code              = supervisor read, page not present
> ...
> I have a crashdump that shows:
> #6  0xc06c9c1b in calltrap () at /usr/src/sys/i386/i386/exception.s:159
> #7  0xc044ecd0 in pf_state_tree_lan_ext_RB_REMOVE_COLOR (head=0xc2a256a8,
>     parent=0xc442c6a0, elm=0xc40aa8e0) at
> /usr/src/sys/contrib/pf/net/pf.c:391 #8  0xc044ef79 in
> pf_state_tree_lan_ext_RB_REMOVE (head=0xc2a256a8, elm=0xc404a11c) at
> /usr/src/sys/contrib/pf/net/pf.c:391
> #9  0xc045383e in pf_unlink_state (cur=0xc404a11c)
>     at /usr/src/sys/contrib/pf/net/pf.c:1158
> #10 0xc0456b6e in pf_purge_expired_states (maxcheck=119)
>     at /usr/src/sys/contrib/pf/net/pf.c:1242
> #11 0xc04570f9 in pf_purge_thread (v=0x0)
>     at /usr/src/sys/contrib/pf/net/pf.c:998
> #12 0xc0535781 in fork_exit (callout=0xc0456f50 <pf_purge_thread>, arg=0x0,
>     frame=0xd2d4cd38) at /usr/src/sys/kern/kern_fork.c:810
> #13 0xc06c9c90 in fork_trampoline () at
> /usr/src/sys/i386/i386/exception.s:264
>
> Working up, 'parent' in pf_state_tree_lan_ext_RB_REMOVE_COLOR() has
> a garbage u.s.entry_lan_ext:
> (kgdb) p parent->u
> $3 = {s = {entry_lan_ext = {rbe_left = 0x10602, rbe_right = 0x50000,
>       rbe_parent = 0xc40aa8e0, rbe_color = -1002258432}, entry_ext_gwy = {
>       rbe_left = 0xc3c42238, rbe_right = 0x1, rbe_parent = 0x0,
>       rbe_color = 0}, entry_id = {rbe_left = 0xc3c54470, rbe_right = 0x0,
>       rbe_parent = 0x0, rbe_color = 0}, entry_list = {tqe_next =
> 0xc41f9e6c, tqe_prev = 0x0}, kif = 0xc442c58c},
>   ifname = "\002\006\001\000\000\000\005\000à¨\nÄ\000ÀBÄ"}
>
> Does anyone have any suggestions on where to look next?

You could try the attached patch that I just set to re@ for inclusion.  There 
is an obvious error in how I handle the pf_consistency_lock in the purge 
thread that might well be the culprit for the error you are seeing.  I assume 
you can't trigger the panic at will, though.  In any case I'd be interested in 
your feedback, thanks.

-- 
/"\  Best regards,                      | mla...@freebsd.org
\ /  Max Laier                          | ICQ #67774661
 X   http://pf4freebsd.love2party.net/  | mla...@efnet
/ \  ASCII Ribbon Campaign              | Against HTML Mail and News
Index: sys/contrib/pf/net/pfvar.h
===================================================================
--- sys/contrib/pf/net/pfvar.h	(revision 196216)
+++ sys/contrib/pf/net/pfvar.h	(working copy)
@@ -1593,8 +1593,13 @@ extern struct pool		 pf_state_pl, pf_altq_pl, pf_p
 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 *,
Index: sys/contrib/pf/net/pf.c
===================================================================
--- sys/contrib/pf/net/pf.c	(revision 196216)
+++ sys/contrib/pf/net/pf.c	(working copy)
@@ -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 *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 maxcheck)
 			/* 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 maxcheck)
 			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 maxcheck)
 		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
 }
_______________________________________________
freebsd-stable@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/freebsd-stable
To unsubscribe, send any mail to "freebsd-stable-unsubscr...@freebsd.org"

Reply via email to