From owner-svn-src-stable-8@FreeBSD.ORG Wed Apr 7 12:42:50 2010 Return-Path: Delivered-To: svn-src-stable-8@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 02724106567A; Wed, 7 Apr 2010 12:42:50 +0000 (UTC) (envelope-from luigi@FreeBSD.org) Received: from svn.freebsd.org (svn.freebsd.org [IPv6:2001:4f8:fff6::2c]) by mx1.freebsd.org (Postfix) with ESMTP id E340B8FC1B; Wed, 7 Apr 2010 12:42:49 +0000 (UTC) Received: from svn.freebsd.org (localhost [127.0.0.1]) by svn.freebsd.org (8.14.3/8.14.3) with ESMTP id o37Cgnxa097835; Wed, 7 Apr 2010 12:42:49 GMT (envelope-from luigi@svn.freebsd.org) Received: (from luigi@localhost) by svn.freebsd.org (8.14.3/8.14.3/Submit) id o37Cgnfs097834; Wed, 7 Apr 2010 12:42:49 GMT (envelope-from luigi@svn.freebsd.org) Message-Id: <201004071242.o37Cgnfs097834@svn.freebsd.org> From: Luigi Rizzo Date: Wed, 7 Apr 2010 12:42:49 +0000 (UTC) To: src-committers@freebsd.org, svn-src-all@freebsd.org, svn-src-stable@freebsd.org, svn-src-stable-8@freebsd.org X-SVN-Group: stable-8 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Cc: Subject: svn commit: r206340 - stable/8/sys/netinet/ipfw X-BeenThere: svn-src-stable-8@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: SVN commit messages for only the 8-stable src tree List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 07 Apr 2010 12:42:50 -0000 Author: luigi Date: Wed Apr 7 12:42:49 2010 New Revision: 206340 URL: http://svn.freebsd.org/changeset/base/206340 Log: fix breakage in ipfw removal. Modified: stable/8/sys/netinet/ipfw/ip_fw_sockopt.c Modified: stable/8/sys/netinet/ipfw/ip_fw_sockopt.c ============================================================================== --- stable/8/sys/netinet/ipfw/ip_fw_sockopt.c Wed Apr 7 08:23:58 2010 (r206339) +++ stable/8/sys/netinet/ipfw/ip_fw_sockopt.c Wed Apr 7 12:42:49 2010 (r206340) @@ -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) + + 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 (n == 0 || map == NULL) { + 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);