svn commit: r193859 - in head/sys: net netgraph netinet netinet/ipfw
Oleg Bulyzhin
oleg at FreeBSD.org
Tue Jun 9 21:27:12 UTC 2009
Author: oleg
Date: Tue Jun 9 21:27:11 2009
New Revision: 193859
URL: http://svn.freebsd.org/changeset/base/193859
Log:
Close long existed race with net.inet.ip.fw.one_pass = 0:
If packet leaves ipfw to other kernel subsystem (dummynet, netgraph, etc)
it carries pointer to matching ipfw rule. If this packet then reinjected back
to ipfw, ruleset processing starts from that rule. If rule was deleted
meanwhile, due to existed race condition panic was possible (as well as
other odd effects like parsing rules in 'reap list').
P.S. this commit changes ABI so userland ipfw related binaries should be
recompiled.
MFC after: 1 month
Tested by: Mikolaj Golub
Modified:
head/sys/net/if_bridge.c
head/sys/net/if_ethersubr.c
head/sys/netgraph/ng_ipfw.c
head/sys/netgraph/ng_ipfw.h
head/sys/netinet/ip_dummynet.h
head/sys/netinet/ip_fw.h
head/sys/netinet/ipfw/ip_dummynet.c
head/sys/netinet/ipfw/ip_fw2.c
head/sys/netinet/ipfw/ip_fw_pfil.c
Modified: head/sys/net/if_bridge.c
==============================================================================
--- head/sys/net/if_bridge.c Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/net/if_bridge.c Tue Jun 9 21:27:11 2009 (r193859)
@@ -3041,11 +3041,19 @@ bridge_pfil(struct mbuf **mp, struct ifn
if (ip_fw_chk_ptr && pfil_ipfw != 0 && dir == PFIL_OUT && ifp != NULL) {
INIT_VNET_INET(curvnet);
+ struct dn_pkt_tag *dn_tag;
error = -1;
- args.rule = ip_dn_claim_rule(*mp);
- if (args.rule != NULL && V_fw_one_pass)
- goto ipfwpass; /* packet already partially processed */
+ dn_tag = ip_dn_claim_tag(*mp);
+ if (dn_tag != NULL) {
+ if (dn_tag->rule != NULL && V_fw_one_pass)
+ /* packet already partially processed */
+ goto ipfwpass;
+ args.rule = dn_tag->rule; /* matching rule to restart */
+ args.rule_id = dn_tag->rule_id;
+ args.chain_id = dn_tag->chain_id;
+ } else
+ args.rule = NULL;
args.m = *mp;
args.oif = ifp;
Modified: head/sys/net/if_ethersubr.c
==============================================================================
--- head/sys/net/if_ethersubr.c Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/net/if_ethersubr.c Tue Jun 9 21:27:11 2009 (r193859)
@@ -145,8 +145,7 @@ MALLOC_DEFINE(M_ARPCOM, "arpcom", "802.*
#if defined(INET) || defined(INET6)
int
-ether_ipfw_chk(struct mbuf **m0, struct ifnet *dst,
- struct ip_fw **rule, int shared);
+ether_ipfw_chk(struct mbuf **m0, struct ifnet *dst, int shared);
#ifdef VIMAGE_GLOBALS
static int ether_ipfw;
#endif
@@ -428,10 +427,9 @@ ether_output_frame(struct ifnet *ifp, st
{
#if defined(INET) || defined(INET6)
INIT_VNET_NET(ifp->if_vnet);
- struct ip_fw *rule = ip_dn_claim_rule(m);
if (ip_fw_chk_ptr && V_ether_ipfw != 0) {
- if (ether_ipfw_chk(&m, ifp, &rule, 0) == 0) {
+ if (ether_ipfw_chk(&m, ifp, 0) == 0) {
if (m) {
m_freem(m);
return EACCES; /* pkt dropped */
@@ -455,8 +453,7 @@ ether_output_frame(struct ifnet *ifp, st
* ether_output_frame.
*/
int
-ether_ipfw_chk(struct mbuf **m0, struct ifnet *dst,
- struct ip_fw **rule, int shared)
+ether_ipfw_chk(struct mbuf **m0, struct ifnet *dst, int shared)
{
INIT_VNET_INET(dst->if_vnet);
struct ether_header *eh;
@@ -464,9 +461,19 @@ ether_ipfw_chk(struct mbuf **m0, struct
struct mbuf *m;
int i;
struct ip_fw_args args;
+ struct dn_pkt_tag *dn_tag;
- if (*rule != NULL && V_fw_one_pass)
- return 1; /* dummynet packet, already partially processed */
+ dn_tag = ip_dn_claim_tag(*m0);
+
+ if (dn_tag != NULL) {
+ if (dn_tag->rule != NULL && V_fw_one_pass)
+ /* dummynet packet, already partially processed */
+ return (1);
+ args.rule = dn_tag->rule; /* matching rule to restart */
+ args.rule_id = dn_tag->rule_id;
+ args.chain_id = dn_tag->chain_id;
+ } else
+ args.rule = NULL;
/*
* I need some amt of data to be contiguous, and in case others need
@@ -487,7 +494,6 @@ ether_ipfw_chk(struct mbuf **m0, struct
args.m = m; /* the packet we are looking at */
args.oif = dst; /* destination, if any */
- args.rule = *rule; /* matching rule to restart */
args.next_hop = NULL; /* we do not support forward yet */
args.eh = &save_eh; /* MAC header for bridged/MAC packets */
args.inp = NULL; /* used by ipfw uid/gid/jail rules */
@@ -508,7 +514,6 @@ ether_ipfw_chk(struct mbuf **m0, struct
ETHER_HDR_LEN);
}
*m0 = m;
- *rule = args.rule;
if (i == IP_FW_DENY) /* drop */
return 0;
@@ -765,9 +770,7 @@ ether_demux(struct ifnet *ifp, struct mb
* Do not do this for PROMISC frames in case we are re-entered.
*/
if (ip_fw_chk_ptr && V_ether_ipfw != 0 && !(m->m_flags & M_PROMISC)) {
- struct ip_fw *rule = ip_dn_claim_rule(m);
-
- if (ether_ipfw_chk(&m, NULL, &rule, 0) == 0) {
+ if (ether_ipfw_chk(&m, NULL, 0) == 0) {
if (m)
m_freem(m); /* dropped; free mbuf chain */
return; /* consumed */
Modified: head/sys/netgraph/ng_ipfw.c
==============================================================================
--- head/sys/netgraph/ng_ipfw.c Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/netgraph/ng_ipfw.c Tue Jun 9 21:27:11 2009 (r193859)
@@ -293,6 +293,8 @@ ng_ipfw_input(struct mbuf **m0, int dir,
return (ENOMEM);
}
ngit->rule = fwa->rule;
+ ngit->rule_id = fwa->rule_id;
+ ngit->chain_id = fwa->chain_id;
ngit->dir = dir;
ngit->ifp = fwa->oif;
m_tag_prepend(m, &ngit->mt);
Modified: head/sys/netgraph/ng_ipfw.h
==============================================================================
--- head/sys/netgraph/ng_ipfw.h Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/netgraph/ng_ipfw.h Tue Jun 9 21:27:11 2009 (r193859)
@@ -38,6 +38,8 @@ extern ng_ipfw_input_t *ng_ipfw_input_p;
struct ng_ipfw_tag {
struct m_tag mt; /* tag header */
struct ip_fw *rule; /* matching rule */
+ uint32_t rule_id; /* matching rule id */
+ uint32_t chain_id; /* ruleset id */
struct ifnet *ifp; /* interface, for ip_output */
int dir;
#define NG_IPFW_OUT 0
Modified: head/sys/netinet/ip_dummynet.h
==============================================================================
--- head/sys/netinet/ip_dummynet.h Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/netinet/ip_dummynet.h Tue Jun 9 21:27:11 2009 (r193859)
@@ -113,6 +113,8 @@ struct dn_heap {
*/
struct dn_pkt_tag {
struct ip_fw *rule; /* matching rule */
+ uint32_t rule_id; /* matching rule id */
+ uint32_t chain_id; /* ruleset id */
int dn_dir; /* action when packet comes out. */
#define DN_TO_IP_OUT 1
#define DN_TO_IP_IN 2
@@ -375,16 +377,16 @@ SLIST_HEAD(dn_pipe_head, dn_pipe);
#ifdef _KERNEL
/*
- * Return the IPFW rule associated with the dummynet tag; if any.
+ * Return the dummynet tag; if any.
* Make sure that the dummynet tag is not reused by lower layers.
*/
-static __inline struct ip_fw *
-ip_dn_claim_rule(struct mbuf *m)
+static __inline struct dn_pkt_tag *
+ip_dn_claim_tag(struct mbuf *m)
{
struct m_tag *mtag = m_tag_find(m, PACKET_TAG_DUMMYNET, NULL);
if (mtag != NULL) {
mtag->m_tag_id = PACKET_TAG_NONE;
- return (((struct dn_pkt_tag *)(mtag+1))->rule);
+ return ((struct dn_pkt_tag *)(mtag + 1));
} else
return (NULL);
}
Modified: head/sys/netinet/ip_fw.h
==============================================================================
--- head/sys/netinet/ip_fw.h Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/netinet/ip_fw.h Tue Jun 9 21:27:11 2009 (r193859)
@@ -465,17 +465,18 @@ struct ip_fw {
struct ip_fw *next_rule; /* ptr to next [skipto] rule */
/* 'next_rule' is used to pass up 'set_disable' status */
- u_int16_t act_ofs; /* offset of action in 32-bit units */
- u_int16_t cmd_len; /* # of 32-bit words in cmd */
- u_int16_t rulenum; /* rule number */
- u_int8_t set; /* rule set (0..31) */
+ uint16_t act_ofs; /* offset of action in 32-bit units */
+ uint16_t cmd_len; /* # of 32-bit words in cmd */
+ uint16_t rulenum; /* rule number */
+ uint8_t set; /* rule set (0..31) */
#define RESVD_SET 31 /* set for default and persistent rules */
- u_int8_t _pad; /* padding */
+ uint8_t _pad; /* padding */
+ uint32_t id; /* rule id */
/* These fields are present in all rules. */
- u_int64_t pcnt; /* Packet counter */
- u_int64_t bcnt; /* Byte counter */
- u_int32_t timestamp; /* tv_sec of last match */
+ uint64_t pcnt; /* Packet counter */
+ uint64_t bcnt; /* Byte counter */
+ uint32_t timestamp; /* tv_sec of last match */
ipfw_insn cmd[1]; /* storage for commands */
};
@@ -619,10 +620,12 @@ struct ip_fw_args {
struct ifnet *oif; /* output interface */
struct sockaddr_in *next_hop; /* forward address */
struct ip_fw *rule; /* matching rule */
+ uint32_t rule_id; /* matching rule id */
+ uint32_t chain_id; /* ruleset id */
struct ether_header *eh; /* for bridged packets */
struct ipfw_flow_id f_id; /* grabbed from IP header */
- u_int32_t cookie; /* a cookie depending on rule action */
+ uint32_t cookie; /* a cookie depending on rule action */
struct inpcb *inp;
struct _ip6dn_args dummypar; /* dummynet->ip6_output */
@@ -662,6 +665,7 @@ struct ip_fw_chain {
LIST_HEAD(, cfg_nat) nat; /* list of nat entries */
struct radix_node_head *tables[IPFW_TABLES_MAX];
struct rwlock rwmtx;
+ uint32_t id; /* ruleset id */
};
#ifdef IPFW_INTERNAL
Modified: head/sys/netinet/ipfw/ip_dummynet.c
==============================================================================
--- head/sys/netinet/ipfw/ip_dummynet.c Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/netinet/ipfw/ip_dummynet.c Tue Jun 9 21:27:11 2009 (r193859)
@@ -243,7 +243,6 @@ static void dummynet_flush(void);
static void dummynet_send(struct mbuf *);
void dummynet_drain(void);
static int dummynet_io(struct mbuf **, int , struct ip_fw_args *);
-static void dn_rule_delete(void *);
/*
* Heap management functions.
@@ -1399,6 +1398,8 @@ dummynet_io(struct mbuf **m0, int dir, s
* Build and enqueue packet + parameters.
*/
pkt->rule = fwa->rule;
+ pkt->rule_id = fwa->rule_id;
+ pkt->chain_id = fwa->chain_id;
pkt->dn_dir = dir;
pkt->ifp = fwa->oif;
@@ -1622,60 +1623,6 @@ dummynet_flush(void)
DUMMYNET_UNLOCK();
}
-extern struct ip_fw *ip_fw_default_rule ;
-static void
-dn_rule_delete_fs(struct dn_flow_set *fs, void *r)
-{
- int i ;
- struct dn_flow_queue *q ;
- struct mbuf *m ;
-
- for (i = 0 ; i <= fs->rq_size ; i++) /* last one is ovflow */
- for (q = fs->rq[i] ; q ; q = q->next )
- for (m = q->head ; m ; m = m->m_nextpkt ) {
- struct dn_pkt_tag *pkt = dn_tag_get(m) ;
- if (pkt->rule == r)
- pkt->rule = ip_fw_default_rule ;
- }
-}
-
-/*
- * When a firewall rule is deleted, scan all queues and remove the pointer
- * to the rule from matching packets, making them point to the default rule.
- * The pointer is used to reinject packets in case one_pass = 0.
- */
-void
-dn_rule_delete(void *r)
-{
- struct dn_pipe *pipe;
- struct dn_flow_set *fs;
- struct dn_pkt_tag *pkt;
- struct mbuf *m;
- int i;
-
- DUMMYNET_LOCK();
- /*
- * If the rule references a queue (dn_flow_set), then scan
- * the flow set, otherwise scan pipes. Should do either, but doing
- * both does not harm.
- */
- for (i = 0; i < HASHSIZE; i++)
- SLIST_FOREACH(fs, &flowsethash[i], next)
- dn_rule_delete_fs(fs, r);
-
- for (i = 0; i < HASHSIZE; i++)
- SLIST_FOREACH(pipe, &pipehash[i], next) {
- fs = &(pipe->fs);
- dn_rule_delete_fs(fs, r);
- for (m = pipe->head ; m ; m = m->m_nextpkt ) {
- pkt = dn_tag_get(m);
- if (pkt->rule == r)
- pkt->rule = ip_fw_default_rule;
- }
- }
- DUMMYNET_UNLOCK();
-}
-
/*
* setup RED parameters
*/
@@ -2299,7 +2246,6 @@ ip_dn_init(void)
ip_dn_ctl_ptr = ip_dn_ctl;
ip_dn_io_ptr = dummynet_io;
- ip_dn_ruledel_ptr = dn_rule_delete;
TASK_INIT(&dn_task, 0, dummynet_task, NULL);
dn_tq = taskqueue_create_fast("dummynet", M_NOWAIT,
@@ -2319,7 +2265,6 @@ ip_dn_destroy(void)
{
ip_dn_ctl_ptr = NULL;
ip_dn_io_ptr = NULL;
- ip_dn_ruledel_ptr = NULL;
DUMMYNET_LOCK();
callout_stop(&dn_timeout);
Modified: head/sys/netinet/ipfw/ip_fw2.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw2.c Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/netinet/ipfw/ip_fw2.c Tue Jun 9 21:27:11 2009 (r193859)
@@ -132,6 +132,8 @@ static int default_to_accept;
#endif
static uma_zone_t ipfw_dyn_rule_zone;
+struct ip_fw *ip_fw_default_rule;
+
/*
* Data structure to cache our ucred related
* information. This structure only gets used if
@@ -2520,9 +2522,21 @@ do { \
if (args->rule) {
/*
* Packet has already been tagged. Look for the next rule
- * to restart processing.
+ * to restart processing. Make sure that args->rule still
+ * exists and not changed.
*/
- f = args->rule->next_rule;
+ if (chain->id != args->chain_id) {
+ for (f = chain->rules; f != NULL; f = f->next)
+ if (f == args->rule && f->id == args->rule_id)
+ break;
+
+ if (f != NULL)
+ f = f->next_rule;
+ else
+ f = ip_fw_default_rule;
+ } else
+ f = args->rule->next_rule;
+
if (f == NULL)
f = lookup_next_rule(args->rule, 0);
} else {
@@ -3234,6 +3248,8 @@ check_body:
case O_PIPE:
case O_QUEUE:
args->rule = f; /* report matching rule */
+ args->rule_id = f->id;
+ args->chain_id = chain->id;
if (cmd->arg1 == IP_FW_TABLEARG)
args->cookie = tablearg;
else
@@ -3342,6 +3358,8 @@ check_body:
case O_NETGRAPH:
case O_NGTEE:
args->rule = f; /* report matching rule */
+ args->rule_id = f->id;
+ args->chain_id = chain->id;
if (cmd->arg1 == IP_FW_TABLEARG)
args->cookie = tablearg;
else
@@ -3364,6 +3382,8 @@ check_body:
if (IPFW_NAT_LOADED) {
args->rule = f; /* Report matching rule. */
+ args->rule_id = f->id;
+ args->chain_id = chain->id;
t = ((ipfw_insn_nat *)cmd)->nat;
if (t == NULL) {
nat_id = (cmd->arg1 == IP_FW_TABLEARG) ?
@@ -3422,6 +3442,8 @@ check_body:
ip->ip_sum = in_cksum(m, hlen);
retval = IP_FW_REASS;
args->rule = f;
+ args->rule_id = f->id;
+ args->chain_id = chain->id;
goto done;
} else {
retval = IP_FW_DENY;
@@ -3480,6 +3502,8 @@ flush_rule_ptrs(struct ip_fw_chain *chai
IPFW_WLOCK_ASSERT(chain);
+ chain->id++;
+
for (rule = chain->rules; rule; rule = rule->next)
rule->next_rule = NULL;
}
@@ -3516,6 +3540,7 @@ add_rule(struct ip_fw_chain *chain, stru
if (chain->rules == NULL) { /* default rule */
chain->rules = rule;
+ rule->id = ++chain->id;
goto done;
}
@@ -3557,6 +3582,8 @@ add_rule(struct ip_fw_chain *chain, stru
}
}
flush_rule_ptrs(chain);
+ /* chain->id incremented inside flush_rule_ptrs() */
+ rule->id = chain->id;
done:
V_static_count++;
V_static_len += l;
@@ -3602,12 +3629,6 @@ remove_rule(struct ip_fw_chain *chain, s
}
/*
- * Hook for cleaning up dummynet when an ipfw rule is deleted.
- * Set/cleared when dummynet module is loaded/unloaded.
- */
-void (*ip_dn_ruledel_ptr)(void *) = NULL;
-
-/**
* Reclaim storage associated with a list of rules. This is
* typically the list created using remove_rule.
*/
@@ -3618,8 +3639,6 @@ reap_rules(struct ip_fw *head)
while ((rule = head) != NULL) {
head = head->next;
- if (ip_dn_ruledel_ptr)
- ip_dn_ruledel_ptr(rule);
free(rule, M_IPFW);
}
}
@@ -4521,15 +4540,6 @@ ipfw_ctl(struct sockopt *sopt)
#undef RULE_MAXSIZE
}
-/**
- * dummynet needs a reference to the default rule, because rules can be
- * deleted while packets hold a reference to them. When this happens,
- * dummynet changes the reference to the default rule (it could well be a
- * NULL pointer, but this way we do not need to check for the special
- * case, plus here he have info on the default behaviour).
- */
-struct ip_fw *ip_fw_default_rule;
-
/*
* This procedure is only used to handle keepalives. It is invoked
* every dyn_keepalive_period
Modified: head/sys/netinet/ipfw/ip_fw_pfil.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw_pfil.c Tue Jun 9 21:17:57 2009 (r193858)
+++ head/sys/netinet/ipfw/ip_fw_pfil.c Tue Jun 9 21:27:11 2009 (r193859)
@@ -113,6 +113,8 @@ ipfw_check_in(void *arg, struct mbuf **m
KASSERT(ng_tag->dir == NG_IPFW_IN,
("ng_ipfw tag with wrong direction"));
args.rule = ng_tag->rule;
+ args.rule_id = ng_tag->rule_id;
+ args.chain_id = ng_tag->chain_id;
m_tag_delete(*m0, (struct m_tag *)ng_tag);
}
@@ -123,6 +125,8 @@ again:
dt = (struct dn_pkt_tag *)(dn_tag+1);
args.rule = dt->rule;
+ args.rule_id = dt->rule_id;
+ args.chain_id = dt->chain_id;
m_tag_delete(*m0, dn_tag);
}
@@ -243,6 +247,8 @@ ipfw_check_out(void *arg, struct mbuf **
KASSERT(ng_tag->dir == NG_IPFW_OUT,
("ng_ipfw tag with wrong direction"));
args.rule = ng_tag->rule;
+ args.rule_id = ng_tag->rule_id;
+ args.chain_id = ng_tag->chain_id;
m_tag_delete(*m0, (struct m_tag *)ng_tag);
}
@@ -253,6 +259,8 @@ again:
dt = (struct dn_pkt_tag *)(dn_tag+1);
args.rule = dt->rule;
+ args.rule_id = dt->rule_id;
+ args.chain_id = dt->chain_id;
m_tag_delete(*m0, dn_tag);
}
More information about the svn-src-all
mailing list