From 933e605154c439218f73f48b028abbeed336c3c5 Mon Sep 17 00:00:00 2001 From: Phil Sutter Date: Wed, 31 Jan 2024 18:08:43 +0100 Subject: xshared: Fix for memleak in option merging with ebtables The crucial difference in ebtables is that all extensions are loaded up front instead of while parsing -m/-j flags. Since this loading of all extensions before every call to do_parse() is pointless overhead (cf. ebtables-restore), other tools' mechanism of freeing all merged options in xtables_free_opts() after handling each command and resetting xt_params->opts at the start of the parser loop is problematic. Fixed commit entailed a hack to defeat the xt_params->opts happening at start of do_parse() by assigning to xt_params->orig_opts after loading all extensions. This approach caused a memleak though since xtables_free_opts() called from xtables_merge_options() will free the opts pointer only if it differs from orig_opts. Resolve this via a different approach which eliminates the xt_params->opts reset at the start of do_parse(): Make xt_params->opts be NULL until the first extension is loaded. Option merging in command_match() and command_jump() tolerates a NULL pointer there after minimal adjustment. Deinit in xtables_free_opts() is already fine as it (re)turns xt_params->opts to a NULL pointer. With do_parse() expecting that and falling back to xt_params->orig_opts, no explicit initialization is required anymore and thus ebtables' init is not mangled by accident. A critical part is that do_parse() checks xt_params->opts pointer upon each call to getopt_long() as it may get assigned while parsing. Fixes: 58d364c7120b5 ("ebtables: Use do_parse() from xshared") Signed-off-by: Phil Sutter --- iptables/xshared.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) (limited to 'iptables/xshared.c') diff --git a/iptables/xshared.c b/iptables/xshared.c index 43fa929d..7d073891 100644 --- a/iptables/xshared.c +++ b/iptables/xshared.c @@ -798,6 +798,9 @@ static void command_match(struct iptables_command_state *cs, bool invert) else if (m->extra_opts != NULL) opts = xtables_merge_options(xt_params->orig_opts, opts, m->extra_opts, &m->option_offset); + else + return; + if (opts == NULL) xtables_error(OTHER_PROBLEM, "can't alloc memory!"); xt_params->opts = opts; @@ -856,10 +859,13 @@ void command_jump(struct iptables_command_state *cs, const char *jumpto) opts = xtables_options_xfrm(xt_params->orig_opts, opts, cs->target->x6_options, &cs->target->option_offset); - else + else if (cs->target->extra_opts != NULL) opts = xtables_merge_options(xt_params->orig_opts, opts, cs->target->extra_opts, &cs->target->option_offset); + else + return; + if (opts == NULL) xtables_error(OTHER_PROBLEM, "can't alloc memory!"); xt_params->opts = opts; @@ -1484,10 +1490,10 @@ void do_parse(int argc, char *argv[], demand-load a protocol. */ opterr = 0; - xt_params->opts = xt_params->orig_opts; while ((cs->c = getopt_long(argc, argv, optstring_lookup(afinfo->family), - xt_params->opts, NULL)) != -1) { + xt_params->opts ?: xt_params->orig_opts, + NULL)) != -1) { switch (cs->c) { /* * Command selection -- cgit v1.2.3