d/freebsd-nq
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

Eliminate rmlock from ipfw's BPF code.

Browse Source 
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
This commit is contained in:
Andrey V. Elsukov 2019-07-23 12:52:36 +00:00
parent c2dc497a38
commit 455d2ecb71
1 changed files with 14 additions and 41 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

55
sys/netpfil/ipfw/ip_fw_bpf.c
View File

@ -32,7 +32,6 @@ __FBSDID("$FreeBSD$");
#include <sys/mbuf.h>
#include <sys/kernel.h>
#include <sys/lock.h>
#include <sys/rmlock.h>
#include <sys/socket.h>
#include <net/ethernet.h>
#include <net/if.h>
@ -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, caddr_t params)
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, caddr_t params)
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();
}