From owner-svn-src-all@FreeBSD.ORG Wed Oct 19 13:13:57 2011 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 0AC9C106564A; Wed, 19 Oct 2011 13:13:57 +0000 (UTC) (envelope-from bz@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id E40C08FC0C; Wed, 19 Oct 2011 13:13:56 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.4/8.14.4) with ESMTP id p9JDDujU026275; Wed, 19 Oct 2011 13:13:56 GMT (envelope-from bz@svn.freebsd.org) Received: (from bz@localhost) by svn.freebsd.org (8.14.4/8.14.4/Submit) id p9JDDuDP026273; Wed, 19 Oct 2011 13:13:56 GMT (envelope-from bz@svn.freebsd.org) Message-Id: <201110191313.p9JDDuDP026273@svn.freebsd.org> From: "Bjoern A. Zeeb" Date: Wed, 19 Oct 2011 13:13:56 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r226544 - head/sys/contrib/pf/net X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 19 Oct 2011 13:13:57 -0000 Author: bz Date: Wed Oct 19 13:13:56 2011 New Revision: 226544 URL: http://svn.freebsd.org/changeset/base/226544 Log: Fix recursive pf locking leading to panics. Splatter PF_LOCK_ASSERT()s to document where we are expecting to be called with a lock held to more easily catch unnoticed code paths. This does not neccessarily improve locking in pfsync, it just tries to avoid the panics reported. PR: kern/159390, kern/158873 Submitted by: pluknet (at least something that partly resembles my patch ignoring other cleanup, which I only saw too late on the 2nd PR) MFC After: 3 days Modified: head/sys/contrib/pf/net/if_pfsync.c Modified: head/sys/contrib/pf/net/if_pfsync.c ============================================================================== --- head/sys/contrib/pf/net/if_pfsync.c Wed Oct 19 13:11:50 2011 (r226543) +++ head/sys/contrib/pf/net/if_pfsync.c Wed Oct 19 13:13:56 2011 (r226544) @@ -714,6 +714,8 @@ pfsync_state_import(struct pfsync_state int pool_flags; int error; + PF_LOCK_ASSERT(); + #ifdef __FreeBSD__ if (sp->creatorid == 0 && V_pf_status.debug >= PF_DEBUG_MISC) { #else @@ -1469,7 +1471,9 @@ pfsync_in_ureq(struct pfsync_pkt *pkt, s if (ISSET(st->state_flags, PFSTATE_NOSYNC)) continue; + PF_LOCK(); pfsync_update_state_req(st); + PF_UNLOCK(); } } @@ -2625,6 +2629,8 @@ pfsync_request_update(u_int32_t creatori size_t nlen = sizeof(struct pfsync_upd_req); int s; + PF_LOCK_ASSERT(); + /* * this code does nothing to prevent multiple update requests for the * same state being generated. @@ -2670,6 +2676,8 @@ pfsync_update_state_req(struct pf_state struct pfsync_softc *sc = pfsyncif; #endif + PF_LOCK_ASSERT(); + if (sc == NULL) panic("pfsync_update_state_req: nonexistant instance"); @@ -2801,6 +2809,8 @@ pfsync_q_ins(struct pf_state *st, int q) size_t nlen = pfsync_qs[q].len; int s; + PF_LOCK_ASSERT(); + #ifdef __FreeBSD__ KASSERT(st->sync_state == PFSYNC_S_NONE, ("%s: st->sync_state == PFSYNC_S_NONE", __FUNCTION__)); @@ -2825,13 +2835,7 @@ pfsync_q_ins(struct pf_state *st, int q) if (sc->sc_len + nlen > sc->sc_if.if_mtu) { #endif s = splnet(); -#ifdef __FreeBSD__ - PF_LOCK(); -#endif pfsync_sendout(); -#ifdef __FreeBSD__ - PF_UNLOCK(); -#endif splx(s); nlen = sizeof(struct pfsync_subheader) + pfsync_qs[q].len; @@ -2888,7 +2892,9 @@ pfsync_update_tdb(struct tdb *t, int out if (sc->sc_len + nlen > sc->sc_if.if_mtu) { s = splnet(); + PF_LOCK(); pfsync_sendout(); + PF_UNLOCK(); splx(s); nlen = sizeof(struct pfsync_subheader) + @@ -2991,8 +2997,10 @@ pfsync_bulk_start(void) #endif printf("pfsync: received bulk update request\n"); + PF_LOCK(); pfsync_bulk_status(PFSYNC_BUS_START); pfsync_bulk_update(sc); + PF_UNLOCK(); } void @@ -3003,10 +3011,11 @@ pfsync_bulk_update(void *arg) int i = 0; int s; + PF_LOCK_ASSERT(); + s = splsoftnet(); #ifdef __FreeBSD__ CURVNET_SET(sc->sc_ifp->if_vnet); - PF_LOCK(); #endif do { if (st->sync_state == PFSYNC_S_NONE && @@ -3043,7 +3052,6 @@ pfsync_bulk_update(void *arg) out: #ifdef __FreeBSD__ - PF_UNLOCK(); CURVNET_RESTORE(); #endif splx(s); @@ -3063,6 +3071,8 @@ pfsync_bulk_status(u_int8_t status) struct pfsync_softc *sc = pfsyncif; #endif + PF_LOCK_ASSERT(); + bzero(&r, sizeof(r)); r.subh.action = PFSYNC_ACT_BUS; @@ -3096,7 +3106,9 @@ pfsync_bulk_fail(void *arg) #else timeout_add_sec(&sc->sc_bulkfail_tmo, 5); #endif + PF_LOCK(); pfsync_request_update(0, 0); + PF_UNLOCK(); } else { /* Pretend like the transfer was ok */ sc->sc_ureq_sent = 0; @@ -3139,19 +3151,15 @@ pfsync_send_plus(void *plus, size_t plus #endif int s; + PF_LOCK_ASSERT(); + #ifdef __FreeBSD__ if (sc->sc_len + pluslen > sc->sc_ifp->if_mtu) { #else if (sc->sc_len + pluslen > sc->sc_if.if_mtu) { #endif s = splnet(); -#ifdef __FreeBSD__ - PF_LOCK(); -#endif pfsync_sendout(); -#ifdef __FreeBSD__ - PF_UNLOCK(); -#endif splx(s); } @@ -3159,13 +3167,7 @@ pfsync_send_plus(void *plus, size_t plus sc->sc_len += (sc->sc_pluslen = pluslen); s = splnet(); -#ifdef __FreeBSD__ - PF_LOCK(); -#endif pfsync_sendout(); -#ifdef __FreeBSD__ - PF_UNLOCK(); -#endif splx(s); }