Date: Sat, 15 Aug 2009 00:38:49 +0200 From: Max Laier <max@love2party.net> To: freebsd-stable@freebsd.org Cc: Peter Jeremy <peterjeremy@optushome.com.au> Subject: Re: Panic due to junk pointer in pf(4) Message-ID: <200908150038.50281.max@love2party.net> In-Reply-To: <20090812191609.GA60973@server.vk2pj.dyndns.org> References: <20090812191609.GA60973@server.vk2pj.dyndns.org>
next in thread | previous in thread | raw e-mail | index | archive | help
[-- Attachment #1 --]
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, | mlaier@freebsd.org
\ / Max Laier | ICQ #67774661
X http://pf4freebsd.love2party.net/ | mlaier@EFnet
/ \ ASCII Ribbon Campaign | Against HTML Mail and News
[-- Attachment #2 --]
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
}
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?200908150038.50281.max>