From aa7fb04fcf72cf50ba6c490ae1cae30181672004 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Mon, 6 Aug 2018 17:21:56 +0200 Subject: ebtables: Review match/target lookup Since ebtables does not indicate extension use on commandline via '-m' flag as in iptables, loading of matches has to happen prior to commandline parsing. While parsing, the right extension is searched for unknown parameters by passing it to its 'parse' callback and checking if it succeeds. As an unavoidable side-effect, custom data in xtables_targets objects is being altered if the extension parser succeeds. If called multiple times, do_commandeb() leaks memory and fixing this requires to properly treat the above quirk: * Load extensions just once at program startup, thereby reusing the existing ones for several calls of do_commandeb(). * In ebt_cs_clean(), don't free memory which is being reused. Instead reinit custom extension data if it was used in current do_commandeb() call (i.e., it is contained in cs->match_list). On the other hand, target lookup in command_jump() can be simplified a lot: The only target it may have loaded is 'standard', so just load that at as well at program startup and reduce command_jump() to a simple linked list search. Since 'standard' target does not prove a 'parse' callback, a check is necessary when parsing target options. Signed-off-by: Phil Sutter Signed-off-by: Florian Westphal --- iptables/xtables-eb.c | 55 +++++++++++++++------------------------------------ 1 file changed, 16 insertions(+), 39 deletions(-) (limited to 'iptables/xtables-eb.c') diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c index c5c98c33..ac3ecb8e 100644 --- a/iptables/xtables-eb.c +++ b/iptables/xtables-eb.c @@ -380,33 +380,22 @@ static struct option *merge_options(struct option *oldopts, /* * More glue code. */ -static struct xtables_target *command_jump(struct iptables_command_state *cs, - const char *jumpto) +static struct xtables_target *command_jump(const char *jumpto) { struct xtables_target *target; - size_t size; - - /* XTF_TRY_LOAD (may be chain name) */ - target = xtables_find_target(jumpto, XTF_TRY_LOAD); - - if (!target) - return NULL; - - size = XT_ALIGN(sizeof(struct xt_entry_target)) - + target->size; - - target->t = xtables_calloc(1, size); - target->t->u.target_size = size; - snprintf(target->t->u.user.name, - sizeof(target->t->u.user.name), "%s", jumpto); - target->t->u.user.name[sizeof(target->t->u.user.name)-1] = '\0'; - target->t->u.user.revision = target->revision; + unsigned int verdict; - xs_init_target(target); + /* Standard target? */ + if (!ebt_fill_target(jumpto, &verdict)) + jumpto = "standard"; - opts = merge_options(opts, target->extra_opts, &target->option_offset); - if (opts == NULL) - xtables_error(OTHER_PROBLEM, "Can't alloc memory"); + /* For ebtables, all targets are preloaded. Hence it is either in + * xtables_targets or a custom chain to jump to, in which case + * returning NULL is fine. */ + for (target = xtables_targets; target; target = target->next) { + if (!strcmp(target->name, jumpto)) + break; + } return target; } @@ -668,6 +657,7 @@ void ebt_load_match_extensions(void) ebt_load_target("dnat"); ebt_load_target("snat"); ebt_load_target("redirect"); + ebt_load_target("standard"); } void ebt_add_match(struct xtables_match *m, @@ -787,20 +777,6 @@ int do_commandeb(struct nft_handle *h, int argc, char *argv[], char **table, struct xtables_rule_match *xtrm_i; struct ebt_match *match; - if (nft_init(h, xtables_bridge) < 0) - xtables_error(OTHER_PROBLEM, - "Could not initialize nftables layer."); - - h->ops = nft_family_ops_lookup(h->family); - if (h->ops == NULL) - xtables_error(PARAMETER_PROBLEM, "Unknown family"); - - /* manually registering ebt matches, given the original ebtables parser - * don't use '-m matchname' and the match can't loaded dinamically when - * the user calls it. - */ - ebt_load_match_extensions(); - /* clear mflags in case do_commandeb gets called a second time * (we clear the global list of all matches for security)*/ for (m = xtables_matches; m; m = m->next) @@ -1047,7 +1023,7 @@ print_zero: } else if (c == 'j') { ebt_check_option2(&flags, OPT_JUMP); cs.jumpto = parse_target(optarg); - cs.target = command_jump(&cs, cs.jumpto); + cs.target = command_jump(cs.jumpto); break; } else if (c == 's') { ebt_check_option2(&flags, OPT_SOURCE); @@ -1231,7 +1207,8 @@ print_zero: /* Is it a watcher option? */ for (w = xtables_targets; w; w = w->next) { - if (w->parse(c - w->option_offset, argv, + if (w->parse && + w->parse(c - w->option_offset, argv, ebt_invert, &w->tflags, NULL, &w->t)) { ebt_add_watcher(w, &cs); -- cgit v1.2.3