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

Close long existed race with net.inet.ip.fw.one_pass = 0:

Browse Source 
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
This commit is contained in:
Oleg Bulyzhin 2009-06-09 21:27:11 +00:00
parent 15d13a59a3
commit dda10d624c
Notes: svn2git 2020-12-20 02:59:44 +00:00 
svn path=/head/; revision=193859
9 changed files with 89 additions and 105 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

14
sys/net/if_bridge.c
View File

@ -3041,11 +3041,19 @@ bridge_pfil(struct mbuf **mp, struct ifnet *bifp, struct ifnet *ifp, int dir)
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;

29
sys/net/if_ethersubr.c
View File

@ -145,8 +145,7 @@ MALLOC_DEFINE(M_ARPCOM, "arpcom", "802.* interface internals");
#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, struct mbuf *m)
{
#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, struct mbuf *m)
* 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 ifnet *dst,
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 ifnet *dst,
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 ifnet *dst,
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 mbuf *m)
* 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 */

2
sys/netgraph/ng_ipfw.c
View File

@ -293,6 +293,8 @@ ng_ipfw_input(struct mbuf **m0, int dir, struct ip_fw_args *fwa, int tee)
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);

2
sys/netgraph/ng_ipfw.h
View File

@ -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

10
sys/netinet/ip_dummynet.h
View File

@ -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);
}

22
sys/netinet/ip_fw.h
View File

@ -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

59
sys/netinet/ipfw/ip_dummynet.c
View File

@ -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, struct ip_fw_args *fwa)
* 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);

48
sys/netinet/ipfw/ip_fw2.c
View File

@ -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 @@ do { \
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 @@ do { \
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 @@ do { \
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 @@ do { \
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 *chain)
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, struct ip_fw *input_rule)
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, struct ip_fw *input_rule)
}
}
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, struct ip_fw *rule,
}
/*
* 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

8
sys/netinet/ipfw/ip_fw_pfil.c
View File

@ -113,6 +113,8 @@ ipfw_check_in(void *arg, struct mbuf **m0, struct ifnet *ifp, int dir,
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 @@ ipfw_check_in(void *arg, struct mbuf **m0, struct ifnet *ifp, int dir,
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 **m0, struct ifnet *ifp, int dir,
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 @@ ipfw_check_out(void *arg, struct mbuf **m0, struct ifnet *ifp, int dir,
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);
}