From owner-svn-src-head@freebsd.org Tue Jul 23 12:52:36 2019 Return-Path: Delivered-To: svn-src-head@mailman.nyi.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.nyi.freebsd.org (Postfix) with ESMTP id EAE36A931C; Tue, 23 Jul 2019 12:52:36 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from mxrelay.nyi.freebsd.org (mxrelay.nyi.freebsd.org [IPv6:2610:1c1:1:606c::19:3]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits) client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "mxrelay.nyi.freebsd.org", Issuer "Let's Encrypt Authority X3" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id CCF708F743; Tue, 23 Jul 2019 12:52:36 +0000 (UTC) (envelope-from ae@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 mxrelay.nyi.freebsd.org (Postfix) with ESMTPS id A4DA725E48; Tue, 23 Jul 2019 12:52:36 +0000 (UTC) (envelope-from ae@FreeBSD.org) Received: from repo.freebsd.org ([127.0.1.37]) by repo.freebsd.org (8.15.2/8.15.2) with ESMTP id x6NCqaJv018883; Tue, 23 Jul 2019 12:52:36 GMT (envelope-from ae@FreeBSD.org) Received: (from ae@localhost) by repo.freebsd.org (8.15.2/8.15.2/Submit) id x6NCqaS9018882; Tue, 23 Jul 2019 12:52:36 GMT (envelope-from ae@FreeBSD.org) Message-Id: <201907231252.x6NCqaS9018882@repo.freebsd.org> X-Authentication-Warning: repo.freebsd.org: ae set sender to ae@FreeBSD.org using -f From: "Andrey V. Elsukov" Date: Tue, 23 Jul 2019 12:52:36 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-head@freebsd.org Subject: svn commit: r350240 - head/sys/netpfil/ipfw X-SVN-Group: head X-SVN-Commit-Author: ae X-SVN-Commit-Paths: head/sys/netpfil/ipfw X-SVN-Commit-Revision: 350240 X-SVN-Commit-Repository: base MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: CCF708F743 X-Spamd-Bar: -- Authentication-Results: mx1.freebsd.org X-Spamd-Result: default: False [-2.93 / 15.00]; local_wl_from(0.00)[FreeBSD.org]; NEURAL_HAM_MEDIUM(-1.00)[-1.000,0]; NEURAL_HAM_SHORT(-0.93)[-0.929,0]; NEURAL_HAM_LONG(-1.00)[-1.000,0]; ASN(0.00)[asn:11403, ipnet:2610:1c1:1::/48, country:US] X-BeenThere: svn-src-head@freebsd.org X-Mailman-Version: 2.1.29 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: Tue, 23 Jul 2019 12:52:37 -0000 Author: ae Date: Tue Jul 23 12:52:36 2019 New Revision: 350240 URL: https://svnweb.freebsd.org/changeset/base/350240 Log: Eliminate rmlock from ipfw's BPF code. After r343631 pfil hooks are invoked in net_epoch_preempt section, this allows to avoid extra locking. Add NET_EPOCH_ASSER() assertion to each ipfw_bpf_*tap*() call to require to be called from inside epoch section. Use NET_EPOCH_WAIT() in ipfw_clone_destroy() to wait until it becomes safe to free() ifnet. And use on-stack ifnet pointer in each ipfw_bpf_*tap*() call to avoid NULL pointer dereference in case when V_*log_if global variable will become NULL during ipfw_bpf_*tap*() call. Sponsored by: Yandex LLC Modified: head/sys/netpfil/ipfw/ip_fw_bpf.c Modified: head/sys/netpfil/ipfw/ip_fw_bpf.c ============================================================================== --- head/sys/netpfil/ipfw/ip_fw_bpf.c Tue Jul 23 09:39:27 2019 (r350239) +++ head/sys/netpfil/ipfw/ip_fw_bpf.c Tue Jul 23 12:52:36 2019 (r350240) @@ -32,7 +32,6 @@ __FBSDID("$FreeBSD$"); #include #include #include -#include #include #include #include @@ -57,15 +56,6 @@ VNET_DEFINE_STATIC(struct if_clone *, ipfwlog_cloner); #define V_log_if VNET(log_if) #define V_pflog_if VNET(pflog_if) -static struct rmlock log_if_lock; -#define LOGIF_LOCK_INIT(x) rm_init(&log_if_lock, "ipfw log_if lock") -#define LOGIF_LOCK_DESTROY(x) rm_destroy(&log_if_lock) -#define LOGIF_RLOCK_TRACKER struct rm_priotracker _log_tracker -#define LOGIF_RLOCK(x) rm_rlock(&log_if_lock, &_log_tracker) -#define LOGIF_RUNLOCK(x) rm_runlock(&log_if_lock, &_log_tracker) -#define LOGIF_WLOCK(x) rm_wlock(&log_if_lock) -#define LOGIF_WUNLOCK(x) rm_wunlock(&log_if_lock) - static const char ipfwname[] = "ipfw"; static const char ipfwlogname[] = "ipfwlog"; @@ -90,13 +80,12 @@ static void ipfw_clone_destroy(struct ifnet *ifp) { - LOGIF_WLOCK(); if (ifp->if_hdrlen == ETHER_HDR_LEN) V_log_if = NULL; else V_pflog_if = NULL; - LOGIF_WUNLOCK(); + NET_EPOCH_WAIT(); bpfdetach(ifp); if_detach(ifp); if_free(ifp); @@ -118,16 +107,13 @@ ipfw_clone_create(struct if_clone *ifc, int unit, cadd ifp->if_hdrlen = ETHER_HDR_LEN; if_attach(ifp); bpfattach(ifp, DLT_EN10MB, ETHER_HDR_LEN); - LOGIF_WLOCK(); if (V_log_if != NULL) { - LOGIF_WUNLOCK(); bpfdetach(ifp); if_detach(ifp); if_free(ifp); return (EEXIST); } V_log_if = ifp; - LOGIF_WUNLOCK(); return (0); } @@ -147,48 +133,42 @@ ipfwlog_clone_create(struct if_clone *ifc, int unit, c ifp->if_hdrlen = PFLOG_HDRLEN; if_attach(ifp); bpfattach(ifp, DLT_PFLOG, PFLOG_HDRLEN); - LOGIF_WLOCK(); if (V_pflog_if != NULL) { - LOGIF_WUNLOCK(); bpfdetach(ifp); if_detach(ifp); if_free(ifp); return (EEXIST); } V_pflog_if = ifp; - LOGIF_WUNLOCK(); return (0); } void ipfw_bpf_tap(u_char *pkt, u_int pktlen) { - LOGIF_RLOCK_TRACKER; + struct ifnet *ifp = V_log_if; - LOGIF_RLOCK(); - if (V_log_if != NULL) - BPF_TAP(V_log_if, pkt, pktlen); - LOGIF_RUNLOCK(); + NET_EPOCH_ASSERT(); + if (ifp != NULL) + BPF_TAP(ifp, pkt, pktlen); } void ipfw_bpf_mtap(struct mbuf *m) { - LOGIF_RLOCK_TRACKER; + struct ifnet *ifp = V_log_if; - LOGIF_RLOCK(); - if (V_log_if != NULL) - BPF_MTAP(V_log_if, m); - LOGIF_RUNLOCK(); + NET_EPOCH_ASSERT(); + if (ifp != NULL) + BPF_MTAP(ifp, m); } void ipfw_bpf_mtap2(void *data, u_int dlen, struct mbuf *m) { struct ifnet *logif; - LOGIF_RLOCK_TRACKER; - LOGIF_RLOCK(); + NET_EPOCH_ASSERT(); switch (dlen) { case (ETHER_HDR_LEN): logif = V_log_if; @@ -205,19 +185,14 @@ ipfw_bpf_mtap2(void *data, u_int dlen, struct mbuf *m) if (logif != NULL) BPF_MTAP2(logif, data, dlen, m); - - LOGIF_RUNLOCK(); } void -ipfw_bpf_init(int first) +ipfw_bpf_init(int first __unused) { - if (first) { - LOGIF_LOCK_INIT(); - V_log_if = NULL; - V_pflog_if = NULL; - } + V_log_if = NULL; + V_pflog_if = NULL; V_ipfw_cloner = if_clone_simple(ipfwname, ipfw_clone_create, ipfw_clone_destroy, 0); V_ipfwlog_cloner = if_clone_simple(ipfwlogname, ipfwlog_clone_create, @@ -225,12 +200,10 @@ ipfw_bpf_init(int first) } void -ipfw_bpf_uninit(int last) +ipfw_bpf_uninit(int last __unused) { if_clone_detach(V_ipfw_cloner); if_clone_detach(V_ipfwlog_cloner); - if (last) - LOGIF_LOCK_DESTROY(); }