From owner-svn-src-head@freebsd.org Wed Mar 22 21:18:19 2017 Return-Path: Delivered-To: svn-src-head@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id B02A2D11295; Wed, 22 Mar 2017 21:18:19 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org (repo.freebsd.org [IPv6:2610:1c1:1:6068::e6a:0]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id 882F414AA; Wed, 22 Mar 2017 21:18:19 +0000 (UTC) (envelope-from kp@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id v2MLIIGs018313; Wed, 22 Mar 2017 21:18:18 GMT (envelope-from kp@FreeBSD.org) Received: (from kp@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id v2MLIIDC018310; Wed, 22 Mar 2017 21:18:18 GMT (envelope-from kp@FreeBSD.org) Message-Id: <201703222118.v2MLIIDC018310@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: kp set sender to kp@FreeBSD.org using -f From: Kristof Provost Date: Wed, 22 Mar 2017 21:18:18 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r315741 - in head/sys: net netpfil/pf X-SVN-Group: head MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: SVN commit messages for the src tree for head/-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 22 Mar 2017 21:18:19 -0000 Author: kp Date: Wed Mar 22 21:18:18 2017 New Revision: 315741 URL: https://svnweb.freebsd.org/changeset/base/315741 Log: pf: Fix possible shutdown race Prevent possible races in the pf_unload() / pf_purge_thread() shutdown code. Lock the pf_purge_thread() with the new pf_end_lock to prevent these races. Use a shared/exclusive lock, as we need to also acquire another sx lock (VNET_LIST_RLOCK). It's fine for both pf_purge_thread() and pf_unload() to sleep, Pointed out by: eri, glebius, jhb Differential Revision: https://reviews.freebsd.org/D10026 Modified: head/sys/net/pfvar.h head/sys/netpfil/pf/pf.c head/sys/netpfil/pf/pf_ioctl.c Modified: head/sys/net/pfvar.h ============================================================================== --- head/sys/net/pfvar.h Wed Mar 22 20:51:52 2017 (r315740) +++ head/sys/net/pfvar.h Wed Mar 22 21:18:18 2017 (r315741) @@ -154,6 +154,8 @@ extern struct rwlock pf_rules_lock; #define PF_RULES_RASSERT() rw_assert(&pf_rules_lock, RA_RLOCKED) #define PF_RULES_WASSERT() rw_assert(&pf_rules_lock, RA_WLOCKED) +extern struct sx pf_end_lock; + #define PF_MODVER 1 #define PFLOG_MODVER 1 #define PFSYNC_MODVER 1 Modified: head/sys/netpfil/pf/pf.c ============================================================================== --- head/sys/netpfil/pf/pf.c Wed Mar 22 20:51:52 2017 (r315740) +++ head/sys/netpfil/pf/pf.c Wed Mar 22 21:18:18 2017 (r315741) @@ -302,6 +302,7 @@ static void pf_route6(struct mbuf **, int in4_cksum(struct mbuf *m, u_int8_t nxt, int off, int len); extern int pf_end_threads; +extern struct proc *pf_purge_proc; VNET_DEFINE(struct pf_limit, pf_limits[PF_LIMIT_MAX]); @@ -1428,19 +1429,14 @@ pf_purge_thread(void *unused __unused) VNET_ITERATOR_DECL(vnet_iter); u_int idx = 0; - for (;;) { - tsleep(pf_purge_thread, 0, "pftm", hz / 10); + sx_xlock(&pf_end_lock); + while (pf_end_threads == 0) { + sx_sleep(pf_purge_thread, &pf_end_lock, 0, "pftm", hz / 10); VNET_LIST_RLOCK(); VNET_FOREACH(vnet_iter) { CURVNET_SET(vnet_iter); - if (pf_end_threads) { - pf_end_threads++; - wakeup(pf_purge_thread); - VNET_LIST_RUNLOCK(); - kproc_exit(0); - } /* Wait until V_pf_default_rule is initialized. */ if (V_pf_vnet_active == 0) { @@ -1474,7 +1470,10 @@ pf_purge_thread(void *unused __unused) } VNET_LIST_RUNLOCK(); } - /* not reached */ + + pf_end_threads++; + sx_xunlock(&pf_end_lock); + kproc_exit(0); } void Modified: head/sys/netpfil/pf/pf_ioctl.c ============================================================================== --- head/sys/netpfil/pf/pf_ioctl.c Wed Mar 22 20:51:52 2017 (r315740) +++ head/sys/netpfil/pf/pf_ioctl.c Wed Mar 22 21:18:18 2017 (r315741) @@ -198,9 +198,11 @@ VNET_DEFINE(int, pf_vnet_active); #define V_pf_vnet_active VNET(pf_vnet_active) int pf_end_threads; +struct proc *pf_purge_proc; struct rwlock pf_rules_lock; struct sx pf_ioctl_lock; +struct sx pf_end_lock; /* pfsync */ pfsync_state_import_t *pfsync_state_import_ptr = NULL; @@ -3730,6 +3732,7 @@ pf_load(void) rw_init(&pf_rules_lock, "pf rulesets"); sx_init(&pf_ioctl_lock, "pf ioctl"); + sx_init(&pf_end_lock, "pf end thread"); pf_mtag_initialize(); @@ -3738,7 +3741,7 @@ pf_load(void) return (ENOMEM); pf_end_threads = 0; - error = kproc_create(pf_purge_thread, NULL, NULL, 0, 0, "pf purge"); + error = kproc_create(pf_purge_thread, NULL, &pf_purge_proc, 0, 0, "pf purge"); if (error != 0) return (error); @@ -3788,11 +3791,13 @@ pf_unload(void) { int error = 0; + sx_xlock(&pf_end_lock); pf_end_threads = 1; while (pf_end_threads < 2) { wakeup_one(pf_purge_thread); - tsleep(pf_purge_thread, 0, "pftmo", 0); + sx_sleep(pf_purge_proc, &pf_end_lock, 0, "pftmo", 0); } + sx_xunlock(&pf_end_lock); if (pf_dev != NULL) destroy_dev(pf_dev); @@ -3801,6 +3806,7 @@ pf_unload(void) rw_destroy(&pf_rules_lock); sx_destroy(&pf_ioctl_lock); + sx_destroy(&pf_end_lock); return (error); }