Author: luigi
Date: Wed Apr 7 08:23:58 2010
New Revision: 206339
URL: http://svn.freebsd.org/changeset/base/206339
Log:
Hopefully fix the recent breakage in rule deletion.
A few more tests and this will also go into -stable where
the problem is more critical.
Modified:
head/sys/netinet/ipfw/ip_fw_sockopt.c
Modified: head/sys/netinet/ipfw/ip_fw_sockopt.c
==============================================================================
--- head/sys/netinet/ipfw/ip_fw_sockopt.c Wed Apr 7 04:51:19 2010
(r206338)
+++ head/sys/netinet/ipfw/ip_fw_sockopt.c Wed Apr 7 08:23:58 2010
(r206339)
@@ -232,12 +232,49 @@ ipfw_reap_rules(struct ip_fw *head)
}
}
+/*
+ * Used by del_entry() to check if a rule should be kept.
+ * Returns 1 if the rule must be kept, 0 otherwise.
+ *
+ * Called with cmd = {0,1,5}.
+ * cmd == 0 matches on rule numbers, excludes rules in RESVD_SET if n == 0 ;
+ * cmd == 1 matches on set numbers only, rule numbers are ignored;
+ * cmd == 5 matches on rule and set numbers.
+ *
+ * n == 0 is a wildcard for rule numbers, there is no wildcard for sets.
+ *
+ * Rules to keep are
+ * (default || reserved || !match_set || !match_number)
+ * where
+ * default ::= (rule->rulenum == IPFW_DEFAULT_RULE)
+ * // the default rule is always protected
+ *
+ * reserved ::= (cmd == 0 && n == 0 && rule->set == RESVD_SET)
+ * // RESVD_SET is protected only if cmd == 0 and n == 0 ("ipfw flush")
+ *
+ * match_set ::= (cmd == 0 || rule->set == set)
+ * // set number is ignored for cmd == 0
+ *
+ * match_number ::= (cmd == 1 || n == 0 || n == rule->rulenum)
+ * // number is ignored for cmd == 1 or n == 0
+ *
+ */
+static int
+keep_rule(struct ip_fw *rule, uint8_t cmd, uint8_t set, uint32_t n)
+{
+ return
+ (rule->rulenum == IPFW_DEFAULT_RULE) ||
+ (cmd == 0 && n == 0 && rule->set == RESVD_SET) ||
+ !(cmd == 0 || rule->set == set) ||
+ !(cmd == 1 || n == 0 || n == rule->rulenum);
+}
+
/**
- * Remove all rules with given number, and also do set manipulation.
+ * Remove all rules with given number, or do set manipulation.
* Assumes chain != NULL && *chain != NULL.
*
- * The argument is an u_int32_t. The low 16 bit are the rule or set number,
- * the next 8 bits are the new set, the top 8 bits are the command:
+ * The argument is an uint32_t. The low 16 bit are the rule or set number;
+ * the next 8 bits are the new set; the top 8 bits indicate the command:
*
* 0 delete rules numbered "rulenum"
* 1 delete rules in set "rulenum"
@@ -247,26 +284,26 @@ ipfw_reap_rules(struct ip_fw *head)
* 5 delete rules "rulenum" and set "new_set"
*/
static int
-del_entry(struct ip_fw_chain *chain, u_int32_t arg)
+del_entry(struct ip_fw_chain *chain, uint32_t arg)
{
struct ip_fw *rule;
- uint32_t rulenum; /* rule or old_set */
+ uint32_t num; /* rule number or old_set */
uint8_t cmd, new_set;
- int start, end = 0, i, ofs, n;
+ int start, end, i, ofs, n;
struct ip_fw **map = NULL;
int error = 0;
- rulenum = arg & 0xffff;
+ num = arg & 0xffff;
cmd = (arg >> 24) & 0xff;
new_set = (arg >> 16) & 0xff;
if (cmd > 5 || new_set > RESVD_SET)
return EINVAL;
if (cmd == 0 || cmd == 2 || cmd == 5) {
- if (rulenum >= IPFW_DEFAULT_RULE)
+ if (num >= IPFW_DEFAULT_RULE)
return EINVAL;
} else {
- if (rulenum > RESVD_SET) /* old_set */
+ if (num > RESVD_SET) /* old_set */
return EINVAL;
}
@@ -274,20 +311,24 @@ del_entry(struct ip_fw_chain *chain, u_i
chain->reap = NULL; /* prepare for deletions */
switch (cmd) {
- case 0: /* delete rules "rulenum" (rulenum == 0 matches all) */
+ case 0: /* delete rules "num" (num == 0 matches all) */
case 1: /* delete all rules in set N */
case 5: /* delete rules with number N and set "new_set". */
/*
* Locate first rule to delete (start), the rule after
* the last one to delete (end), and count how many
- * rules to delete (n)
+ * rules to delete (n). Always use keep_rule() to
+ * determine which rules to keep.
*/
n = 0;
- if (cmd == 1) { /* look for a specific set, must scan all */
- new_set = rulenum;
- for (start = -1, i = 0; i < chain->n_rules; i++) {
- if (chain->map[i]->set != new_set)
+ if (cmd == 1) {
+ /* look for a specific set including RESVD_SET.
+ * Must scan the entire range, ignore num.
+ */
+ new_set = num;
+ for (start = -1, end = i = 0; i < chain->n_rules; i++) {
+ if (keep_rule(chain->map[i], cmd, new_set, 0))
continue;
if (start < 0)
start = i;
@@ -296,61 +337,54 @@ del_entry(struct ip_fw_chain *chain, u_i
}
end++; /* first non-matching */
} else {
- start = ipfw_find_rule(chain, rulenum, 0);
+ /* Optimized search on rule numbers */
+ start = ipfw_find_rule(chain, num, 0);
for (end = start; end < chain->n_rules; end++) {
rule = chain->map[end];
- if (rulenum > 0 && rule->rulenum != rulenum)
+ if (num > 0 && rule->rulenum != num)
break;
- if (rule->set != RESVD_SET &&
- (cmd == 0 || rule->set == new_set) )
+ if (!keep_rule(rule, cmd, new_set, num))
n++;
}
}
- if (n == 0 && arg == 0)
- break; /* special case, flush on empty ruleset */
- /* allocate the map, if needed */
- if (n > 0)
- map = get_map(chain, -n, 1 /* locked */);
- if (n == 0 || map == NULL) {
+
+ if (n == 0) {
+ /* A flush request (arg == 0) on empty ruleset
+ * returns with no error. On the contrary,
+ * if there is no match on a specific request,
+ * we return EINVAL.
+ */
+ error = (arg == 0) ? 0 : EINVAL;
+ break;
+ }
+
+ /* We have something to delete. Allocate the new map */
+ map = get_map(chain, -n, 1 /* locked */);
+ if (map == NULL) {
error = EINVAL;
break;
}
- /*
- * bcopy the initial part of the map, then individually
- * copy all matching entries between start and end,
- * and then bcopy the final part.
- * Once we are done we can swap maps and clean up the
- * deleted rules (unfortunately we need to repeat a
- * convoluted test). Rules to keep are
- * (set == RESVD_SET || !match_set || !match_rule)
- * where
- * match_set ::= (cmd == 0 || rule->set == new_set)
- * match_rule ::= (cmd == 1 || rule->rulenum == rulenum)
- */
+
+ /* 1. bcopy the initial part of the map */
if (start > 0)
bcopy(chain->map, map, start * sizeof(struct ip_fw *));
+ /* 2. copy active rules between start and end */
for (i = ofs = start; i < end; i++) {
rule = chain->map[i];
- if (rule->set == RESVD_SET ||
- !(cmd == 0 || rule->set == new_set) ||
- !(cmd == 1 || rule->rulenum == rulenum) ) {
- map[ofs++] = chain->map[i];
- }
+ if (keep_rule(rule, cmd, new_set, num))
+ map[ofs++] = rule;
}
+ /* 3. copy the final part of the map */
bcopy(chain->map + end, map + ofs,
(chain->n_rules - end) * sizeof(struct ip_fw *));
-
+ /* 4. swap the maps (under BH_LOCK) */
map = swap_map(chain, map, chain->n_rules - n);
- /* now remove the rules deleted */
+ /* 5. now remove the rules deleted from the old map */
for (i = start; i < end; i++) {
int l;
rule = map[i];
- /* same test as above */
- if (rule->set == RESVD_SET ||
- !(cmd == 0 || rule->set == new_set) ||
- !(cmd == 1 || rule->rulenum == rulenum) )
+ if (keep_rule(rule, cmd, new_set, num))
continue;
-
l = RULESIZE(rule);
chain->static_len -= l;
ipfw_remove_dyn_children(rule);
@@ -359,32 +393,38 @@ del_entry(struct ip_fw_chain *chain, u_i
}
break;
- case 2: /* move rules with given number to new set */
- for (i = 0; i < chain->n_rules; i++) {
+ /*
+ * In the next 3 cases the loop stops at (n_rules - 1)
+ * because the default rule is never eligible..
+ */
+
+ case 2: /* move rules with given RULE number to new set */
+ for (i = 0; i < chain->n_rules - 1; i++) {
rule = chain->map[i];
- if (rule->rulenum == rulenum)
+ if (rule->rulenum == num)
rule->set = new_set;
}
break;
- case 3: /* move rules with given set number to new set */
- for (i = 0; i < chain->n_rules; i++) {
+ case 3: /* move rules with given SET number to new set */
+ for (i = 0; i < chain->n_rules - 1; i++) {
rule = chain->map[i];
- if (rule->set == rulenum)
+ if (rule->set == num)
rule->set = new_set;
}
break;
case 4: /* swap two sets */
- for (i = 0; i < chain->n_rules; i++) {
+ for (i = 0; i < chain->n_rules - 1; i++) {
rule = chain->map[i];
- if (rule->set == rulenum)
+ if (rule->set == num)
rule->set = new_set;
else if (rule->set == new_set)
- rule->set = rulenum;
+ rule->set = num;
}
break;
}
+
rule = chain->reap;
chain->reap = NULL;
IPFW_UH_WUNLOCK(chain);
_______________________________________________
svn-src-head@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
