svn commit: r286014 - in releng/10.2/sys: net netpfil/pf
Gleb Smirnoff
glebius at FreeBSD.org
Wed Jul 29 14:16:27 UTC 2015
Author: glebius
Date: Wed Jul 29 14:16:25 2015
New Revision: 286014
URL: https://svnweb.freebsd.org/changeset/base/286014
Log:
Merge r285939-285941,285943,286004 from stable/10:
- Protect against ioctl() vs ioctl() races.
- Always lock hash row of a source node when updating
its 'states' counter. [1]
- Don't dereference NULL is pf_get_mtag() fails. [2]
- During module unload drop locks before destroying UMA zone.
PR: 182401 [1]
PR: 200222 [2]
Approved by: re (gjb)
Modified:
releng/10.2/sys/net/pfvar.h
releng/10.2/sys/netpfil/pf/pf.c
releng/10.2/sys/netpfil/pf/pf_ioctl.c
Directory Properties:
releng/10.2/ (props changed)
Modified: releng/10.2/sys/net/pfvar.h
==============================================================================
--- releng/10.2/sys/net/pfvar.h Wed Jul 29 14:07:43 2015 (r286013)
+++ releng/10.2/sys/net/pfvar.h Wed Jul 29 14:16:25 2015 (r286014)
@@ -1549,7 +1549,6 @@ extern struct pf_state *pf_find_state_a
extern struct pf_src_node *pf_find_src_node(struct pf_addr *,
struct pf_rule *, sa_family_t, int);
extern void pf_unlink_src_node(struct pf_src_node *);
-extern void pf_unlink_src_node_locked(struct pf_src_node *);
extern u_int pf_free_src_nodes(struct pf_src_node_list *);
extern void pf_print_state(struct pf_state *);
extern void pf_print_flags(u_int8_t);
Modified: releng/10.2/sys/netpfil/pf/pf.c
==============================================================================
--- releng/10.2/sys/netpfil/pf/pf.c Wed Jul 29 14:07:43 2015 (r286013)
+++ releng/10.2/sys/netpfil/pf/pf.c Wed Jul 29 14:16:25 2015 (r286014)
@@ -655,7 +655,10 @@ pf_find_src_node(struct pf_addr *src, st
((af == AF_INET && n->addr.v4.s_addr == src->v4.s_addr) ||
(af == AF_INET6 && bcmp(&n->addr, src, sizeof(*src)) == 0)))
break;
- if (n != NULL || returnlocked == 0)
+ if (n != NULL) {
+ n->states++;
+ PF_HASHROW_UNLOCK(sh);
+ } else if (returnlocked == 0)
PF_HASHROW_UNLOCK(sh);
return (n);
@@ -699,6 +702,7 @@ pf_insert_src_node(struct pf_src_node **
LIST_INSERT_HEAD(&sh->nodes, *sn, entry);
(*sn)->creation = time_uptime;
(*sn)->ruletype = rule->action;
+ (*sn)->states = 1;
if ((*sn)->rule.ptr != NULL)
counter_u64_add((*sn)->rule.ptr->src_nodes, 1);
PF_HASHROW_UNLOCK(sh);
@@ -715,37 +719,13 @@ pf_insert_src_node(struct pf_src_node **
}
void
-pf_unlink_src_node_locked(struct pf_src_node *src)
+pf_unlink_src_node(struct pf_src_node *src)
{
-#ifdef INVARIANTS
- struct pf_srchash *sh;
- sh = &V_pf_srchash[pf_hashsrc(&src->addr, src->af)];
- PF_HASHROW_ASSERT(sh);
-#endif
+ PF_HASHROW_ASSERT(&V_pf_srchash[pf_hashsrc(&src->addr, src->af)]);
LIST_REMOVE(src, entry);
if (src->rule.ptr)
counter_u64_add(src->rule.ptr->src_nodes, -1);
- counter_u64_add(V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1);
-}
-
-void
-pf_unlink_src_node(struct pf_src_node *src)
-{
- struct pf_srchash *sh;
-
- sh = &V_pf_srchash[pf_hashsrc(&src->addr, src->af)];
- PF_HASHROW_LOCK(sh);
- pf_unlink_src_node_locked(src);
- PF_HASHROW_UNLOCK(sh);
-}
-
-static void
-pf_free_src_node(struct pf_src_node *sn)
-{
-
- KASSERT(sn->states == 0, ("%s: %p has refs", __func__, sn));
- uma_zfree(V_pf_sources_z, sn);
}
u_int
@@ -755,10 +735,12 @@ pf_free_src_nodes(struct pf_src_node_lis
u_int count = 0;
LIST_FOREACH_SAFE(sn, head, entry, tmp) {
- pf_free_src_node(sn);
+ uma_zfree(V_pf_sources_z, sn);
count++;
}
+ counter_u64_add(V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], count);
+
return (count);
}
@@ -1550,7 +1532,7 @@ pf_purge_expired_src_nodes()
PF_HASHROW_LOCK(sh);
LIST_FOREACH_SAFE(cur, &sh->nodes, entry, next)
if (cur->states == 0 && cur->expire <= time_uptime) {
- pf_unlink_src_node_locked(cur);
+ pf_unlink_src_node(cur);
LIST_INSERT_HEAD(&freelist, cur, entry);
} else if (cur->rule.ptr != NULL)
cur->rule.ptr->rule_flag |= PFRULE_REFS;
@@ -1565,27 +1547,31 @@ pf_purge_expired_src_nodes()
static void
pf_src_tree_remove_state(struct pf_state *s)
{
- u_int32_t timeout;
+ struct pf_src_node *sn;
+ struct pf_srchash *sh;
+ uint32_t timeout;
+
+ timeout = s->rule.ptr->timeout[PFTM_SRC_NODE] ?
+ s->rule.ptr->timeout[PFTM_SRC_NODE] :
+ V_pf_default_rule.timeout[PFTM_SRC_NODE];
if (s->src_node != NULL) {
+ sn = s->src_node;
+ sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)];
+ PF_HASHROW_LOCK(sh);
if (s->src.tcp_est)
- --s->src_node->conn;
- if (--s->src_node->states == 0) {
- timeout = s->rule.ptr->timeout[PFTM_SRC_NODE];
- if (!timeout)
- timeout =
- V_pf_default_rule.timeout[PFTM_SRC_NODE];
- s->src_node->expire = time_uptime + timeout;
- }
+ --sn->conn;
+ if (--sn->states == 0)
+ sn->expire = time_uptime + timeout;
+ PF_HASHROW_UNLOCK(sh);
}
if (s->nat_src_node != s->src_node && s->nat_src_node != NULL) {
- if (--s->nat_src_node->states == 0) {
- timeout = s->rule.ptr->timeout[PFTM_SRC_NODE];
- if (!timeout)
- timeout =
- V_pf_default_rule.timeout[PFTM_SRC_NODE];
- s->nat_src_node->expire = time_uptime + timeout;
- }
+ sn = s->nat_src_node;
+ sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)];
+ PF_HASHROW_LOCK(sh);
+ if (--sn->states == 0)
+ sn->expire = time_uptime + timeout;
+ PF_HASHROW_UNLOCK(sh);
}
s->src_node = s->nat_src_node = NULL;
}
@@ -3571,15 +3557,12 @@ pf_create_state(struct pf_rule *r, struc
s->creation = time_uptime;
s->expire = time_uptime;
- if (sn != NULL) {
+ if (sn != NULL)
s->src_node = sn;
- s->src_node->states++;
- }
if (nsn != NULL) {
/* XXX We only modify one side for now. */
PF_ACPY(&nsn->raddr, &nk->addr[1], pd->af);
s->nat_src_node = nsn;
- s->nat_src_node->states++;
}
if (pd->proto == IPPROTO_TCP) {
if ((pd->flags & PFDESC_TCP_NORM) && pf_normalize_tcp_init(m,
@@ -3677,14 +3660,32 @@ csfailed:
if (nk != NULL)
uma_zfree(V_pf_state_key_z, nk);
- if (sn != NULL && sn->states == 0 && sn->expire == 0) {
- pf_unlink_src_node(sn);
- pf_free_src_node(sn);
+ if (sn != NULL) {
+ struct pf_srchash *sh;
+
+ sh = &V_pf_srchash[pf_hashsrc(&sn->addr, sn->af)];
+ PF_HASHROW_LOCK(sh);
+ if (--sn->states == 0 && sn->expire == 0) {
+ pf_unlink_src_node(sn);
+ uma_zfree(V_pf_sources_z, sn);
+ counter_u64_add(
+ V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1);
+ }
+ PF_HASHROW_UNLOCK(sh);
}
- if (nsn != sn && nsn != NULL && nsn->states == 0 && nsn->expire == 0) {
- pf_unlink_src_node(nsn);
- pf_free_src_node(nsn);
+ if (nsn != sn && nsn != NULL) {
+ struct pf_srchash *sh;
+
+ sh = &V_pf_srchash[pf_hashsrc(&nsn->addr, nsn->af)];
+ PF_HASHROW_LOCK(sh);
+ if (--nsn->states == 0 && nsn->expire == 0) {
+ pf_unlink_src_node(nsn);
+ uma_zfree(V_pf_sources_z, nsn);
+ counter_u64_add(
+ V_pf_status.scounters[SCNT_SRC_NODE_REMOVALS], 1);
+ }
+ PF_HASHROW_UNLOCK(sh);
}
return (PF_DROP);
@@ -5911,13 +5912,14 @@ done:
((pd.pf_mtag = pf_get_mtag(m)) == NULL)) {
action = PF_DROP;
REASON_SET(&reason, PFRES_MEMORY);
+ } else {
+ if (pqid || (pd.tos & IPTOS_LOWDELAY))
+ pd.pf_mtag->qid = r->pqid;
+ else
+ pd.pf_mtag->qid = r->qid;
+ /* Add hints for ecn. */
+ pd.pf_mtag->hdr = h;
}
- if (pqid || (pd.tos & IPTOS_LOWDELAY))
- pd.pf_mtag->qid = r->pqid;
- else
- pd.pf_mtag->qid = r->qid;
- /* add hints for ecn */
- pd.pf_mtag->hdr = h;
}
#endif /* ALTQ */
@@ -5956,9 +5958,11 @@ done:
log = 1;
DPFPRINTF(PF_DEBUG_MISC,
("pf: failed to allocate tag\n"));
+ } else {
+ pd.pf_mtag->flags |=
+ PF_FASTFWD_OURS_PRESENT;
+ m->m_flags &= ~M_FASTFWD_OURS;
}
- pd.pf_mtag->flags |= PF_FASTFWD_OURS_PRESENT;
- m->m_flags &= ~M_FASTFWD_OURS;
}
ip_divert_ptr(*m0, dir == PF_IN ? DIR_IN : DIR_OUT);
*m0 = NULL;
@@ -6340,13 +6344,14 @@ done:
((pd.pf_mtag = pf_get_mtag(m)) == NULL)) {
action = PF_DROP;
REASON_SET(&reason, PFRES_MEMORY);
+ } else {
+ if (pd.tos & IPTOS_LOWDELAY)
+ pd.pf_mtag->qid = r->pqid;
+ else
+ pd.pf_mtag->qid = r->qid;
+ /* Add hints for ecn. */
+ pd.pf_mtag->hdr = h;
}
- if (pd.tos & IPTOS_LOWDELAY)
- pd.pf_mtag->qid = r->pqid;
- else
- pd.pf_mtag->qid = r->qid;
- /* add hints for ecn */
- pd.pf_mtag->hdr = h;
}
#endif /* ALTQ */
Modified: releng/10.2/sys/netpfil/pf/pf_ioctl.c
==============================================================================
--- releng/10.2/sys/netpfil/pf/pf_ioctl.c Wed Jul 29 14:07:43 2015 (r286013)
+++ releng/10.2/sys/netpfil/pf/pf_ioctl.c Wed Jul 29 14:16:25 2015 (r286014)
@@ -188,6 +188,7 @@ static volatile VNET_DEFINE(int, pf_pfil
VNET_DEFINE(int, pf_end_threads);
struct rwlock pf_rules_lock;
+struct sx pf_ioctl_lock;
/* pfsync */
pfsync_state_import_t *pfsync_state_import_ptr = NULL;
@@ -1090,20 +1091,18 @@ pfioctl(struct cdev *dev, u_long cmd, ca
switch (cmd) {
case DIOCSTART:
- PF_RULES_WLOCK();
+ sx_xlock(&pf_ioctl_lock);
if (V_pf_status.running)
error = EEXIST;
else {
int cpu;
- PF_RULES_WUNLOCK();
error = hook_pf();
if (error) {
DPFPRINTF(PF_DEBUG_MISC,
("pf: pfil registration failed\n"));
break;
}
- PF_RULES_WLOCK();
V_pf_status.running = 1;
V_pf_status.since = time_second;
@@ -1112,27 +1111,23 @@ pfioctl(struct cdev *dev, u_long cmd, ca
DPFPRINTF(PF_DEBUG_MISC, ("pf: started\n"));
}
- PF_RULES_WUNLOCK();
break;
case DIOCSTOP:
- PF_RULES_WLOCK();
+ sx_xlock(&pf_ioctl_lock);
if (!V_pf_status.running)
error = ENOENT;
else {
V_pf_status.running = 0;
- PF_RULES_WUNLOCK();
error = dehook_pf();
if (error) {
V_pf_status.running = 1;
DPFPRINTF(PF_DEBUG_MISC,
("pf: pfil unregistration failed\n"));
}
- PF_RULES_WLOCK();
V_pf_status.since = time_second;
DPFPRINTF(PF_DEBUG_MISC, ("pf: stopped\n"));
}
- PF_RULES_WUNLOCK();
break;
case DIOCADDRULE: {
@@ -3256,6 +3251,8 @@ DIOCCHANGEADDR_error:
break;
}
fail:
+ if (sx_xlocked(&pf_ioctl_lock))
+ sx_xunlock(&pf_ioctl_lock);
CURVNET_RESTORE();
return (error);
@@ -3433,7 +3430,7 @@ pf_kill_srcnodes(struct pfioc_src_node_k
&psnk->psnk_dst.addr.v.a.addr,
&psnk->psnk_dst.addr.v.a.mask,
&sn->raddr, sn->af)) {
- pf_unlink_src_node_locked(sn);
+ pf_unlink_src_node(sn);
LIST_INSERT_HEAD(&kill, sn, entry);
sn->expire = 1;
}
@@ -3446,18 +3443,10 @@ pf_kill_srcnodes(struct pfioc_src_node_k
PF_HASHROW_LOCK(ih);
LIST_FOREACH(s, &ih->states, entry) {
- if (s->src_node && s->src_node->expire == 1) {
-#ifdef INVARIANTS
- s->src_node->states--;
-#endif
+ if (s->src_node && s->src_node->expire == 1)
s->src_node = NULL;
- }
- if (s->nat_src_node && s->nat_src_node->expire == 1) {
-#ifdef INVARIANTS
- s->nat_src_node->states--;
-#endif
+ if (s->nat_src_node && s->nat_src_node->expire == 1)
s->nat_src_node = NULL;
- }
}
PF_HASHROW_UNLOCK(ih);
}
@@ -3728,6 +3717,7 @@ pf_load(void)
VNET_LIST_RUNLOCK();
rw_init(&pf_rules_lock, "pf rulesets");
+ sx_init(&pf_ioctl_lock, "pf ioctl");
pf_dev = make_dev(&pf_cdevsw, 0, 0, 0, 0600, PF_NAME);
if ((error = pfattach()) != 0)
@@ -3741,9 +3731,7 @@ pf_unload(void)
{
int error = 0;
- PF_RULES_WLOCK();
V_pf_status.running = 0;
- PF_RULES_WUNLOCK();
swi_remove(V_pf_swi_cookie);
error = dehook_pf();
if (error) {
@@ -3762,6 +3750,7 @@ pf_unload(void)
wakeup_one(pf_purge_thread);
rw_sleep(pf_purge_thread, &pf_rules_lock, 0, "pftmo", 0);
}
+ PF_RULES_WUNLOCK();
pf_normalize_cleanup();
pfi_cleanup();
pfr_cleanup();
@@ -3769,9 +3758,9 @@ pf_unload(void)
pf_cleanup();
if (IS_DEFAULT_VNET(curvnet))
pf_mtag_cleanup();
- PF_RULES_WUNLOCK();
destroy_dev(pf_dev);
rw_destroy(&pf_rules_lock);
+ sx_destroy(&pf_ioctl_lock);
return (error);
}
More information about the svn-src-releng
mailing list