From: shemminger Date: Thu, 23 Jun 2005 17:36:38 +0000 (+0000) Subject: From: Pablo Neira X-Git-Tag: ss-050808~15 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=6d4662d4f7508fa9d247ab0f5a179946722499fc;p=thirdparty%2Fiproute2.git From: Pablo Neira Hi jamal, I found some spare time to play around a bit more with you ipt action stuff. I've tested the patch attached with the testcase here below. It works fine here. It fixes broken target option checkings (final_check) and a leak in the merge_options function. I've killed copy_options since I didn't find any reason why we need it. --- test.sh --- tc qdisc del dev wlan0 ingress tc qdisc add dev wlan0 ingress tc filter add dev wlan0 parent ffff: protocol ip prio 6 u32 \ match ip src 192.168.0.2/32 flowid 1:16 \ action ipt -j TOS --set-tos Maximize-Reliability sleep 3 tc -s filter ls dev wlan0 parent ffff: --- end of test.sh --- Results: tablename: mangle hook: NF_IP_PRE_ROUTING target: TOS set Maximize-Reliability index 0 filter protocol ip pref 6 u32 filter protocol ip pref 6 u32 fh 800: ht divisor 1 filter protocol ip pref 6 u32 fh 800::800 order 2048 key ht 800 bkt 0 flowid 1:16 match c0a80002/ffffffff at 12 action order 1: tablename: mangle hook: NF_IP_PRE_ROUTING target TOS set Maximize-Reliability index 18 ref 1 bind 1 installed 3 sec used 0 sec Action statistics: Sent 725 bytes 7 pkt (dropped 0, overlimits 0 requeues 0) rate 0bit 0pps backlog 0b 0p requeues 0 Now, check if options passed to the target are correct. # tc filter add dev wlan0 parent ffff: protocol ip prio 6 u32 \ match ip dst 192.168.0.2/32 flowid 1:16 \ action ipt -j TOS --set-tos ^^^ missing parameter ipt: option `--set-tos' requires an argument tc-ipt v0.1: TOS target: Parameter --set-tos is required Try `tc-ipt -h' or 'tc-ipt --help' for more information. btw, how's your schedule ? did you finally get spare time to come to the netfilter workshop in seville ? bye, Pablo --- diff --git a/ChangeLog b/ChangeLog index a9cd316e1..ab4f7a1b4 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2005-06-23 Jamal Hadi Salim + + * Fix for options process with ipt + 2005-06-23 Thomas Graf * Add extended matches (nbyte, cmp, u32, meta) @@ -10,7 +14,7 @@ in batched mode. * Assume stdin if no argument is given to -batch -2005-06-22 Stephen Hemminger +2005-06-22 Stephen Hemminger * Update include files to 2.6.12 * Add ss support for TCP_CONG diff --git a/tc/m_ipt.c b/tc/m_ipt.c index 1d375d31c..ca3955513 100644 --- a/tc/m_ipt.c +++ b/tc/m_ipt.c @@ -69,6 +69,7 @@ static struct option original_opts[] = { }; static struct iptables_target *t_list = NULL; +static struct option *opts = original_opts; static unsigned int global_option_offset = 0; #define OPTION_OFFSET 256 @@ -169,18 +170,13 @@ int string_to_number(const char *s, unsigned int min, unsigned int max, return result; } -static struct option * -copy_options(struct option *oldopts) +static void free_opts(struct option *opts) { - struct option *merge; - unsigned int num_old; - for (num_old = 0; oldopts[num_old].name; num_old++) ; - merge = malloc(sizeof (struct option) * (num_old + 1)); - if (NULL == merge) - return NULL; - memcpy(merge, oldopts, num_old * sizeof (struct option)); - memset(merge + num_old, 0, sizeof (struct option)); - return merge; + if (opts != original_opts) { + free(opts); + opts = original_opts; + global_option_offset = 0; + } } static struct option * @@ -385,7 +381,6 @@ static int parse_ipt(struct action_util *a,int *argc_p, int c; int rargc = *argc_p; char **argv = *argv_p; - struct option *opts; int argc = 0, iargc = 0; char k[16]; int res = -1; @@ -409,11 +404,6 @@ static int parse_ipt(struct action_util *a,int *argc_p, return -1; } - opts = copy_options(original_opts); - - if (NULL == opts) - return -1; - while (1) { c = getopt_long(argc, argv, "j:", opts, NULL); if (c == -1) @@ -440,23 +430,14 @@ static int parse_ipt(struct action_util *a,int *argc_p, default: memset(&fw, 0, sizeof (fw)); if (m) { - unsigned int fake_flags = 0; m->parse(c - m->option_offset, argv, 0, - &fake_flags, NULL, &m->t); + &m->tflags, NULL, &m->t); } else { fprintf(stderr," failed to find target %s\n\n", optarg); return -1; } ok++; - - /*m->final_check(m->t); -- Is this necessary? - ** useful when theres depencies - ** eg ipt_TCPMSS.c has have the TCP match loaded - ** before this can be used; - ** also seems the ECN target needs it - */ - break; } @@ -466,6 +447,7 @@ static int parse_ipt(struct action_util *a,int *argc_p, if (matches(argv[optind], "index") == 0) { if (get_u32(&index, argv[optind + 1], 10)) { fprintf(stderr, "Illegal \"index\"\n"); + free_opts(opts); return -1; } iok++; @@ -479,6 +461,10 @@ static int parse_ipt(struct action_util *a,int *argc_p, return -1; } + /* check that we passed the correct parameters to the target */ + if (m) + m->final_check(m->tflags); + { struct tcmsg *t = NLMSG_DATA(n); if (t->tcm_parent != TC_H_ROOT @@ -519,6 +505,7 @@ static int parse_ipt(struct action_util *a,int *argc_p, *argv_p = argv; optind = 1; + free_opts(opts); return 0; @@ -529,16 +516,10 @@ print_ipt(struct action_util *au,FILE * f, struct rtattr *arg) { struct rtattr *tb[TCA_IPT_MAX + 1]; struct ipt_entry_target *t = NULL; - struct option *opts; if (arg == NULL) return -1; - opts = copy_options(original_opts); - - if (NULL == opts) - return -1; - parse_rtattr_nested(tb, TCA_IPT_MAX, arg); if (tb[TCA_IPT_TABLE] == NULL) { @@ -601,6 +582,7 @@ print_ipt(struct action_util *au,FILE * f, struct rtattr *arg) fprintf(f, " \n"); } + free_opts(opts); return 0; }