diff options
author | Phil Sutter <phil@nwl.cc> | 2024-01-31 18:08:43 +0100 |
---|---|---|
committer | Phil Sutter <phil@nwl.cc> | 2024-02-01 14:51:30 +0100 |
commit | 933e605154c439218f73f48b028abbeed336c3c5 (patch) | |
tree | 1f48566e6378af5fbd3c11c2d2a40f51c1ea9680 /iptables/xtables-eb.c | |
parent | 3a135363c9e73dd51535c58b65afd4be65a4be64 (diff) |
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 <phil@nwl.cc>
Diffstat (limited to 'iptables/xtables-eb.c')
-rw-r--r-- | iptables/xtables-eb.c | 25 |
1 files changed, 13 insertions, 12 deletions
diff --git a/iptables/xtables-eb.c b/iptables/xtables-eb.c index d197b37e..51c699de 100644 --- a/iptables/xtables-eb.c +++ b/iptables/xtables-eb.c @@ -298,11 +298,14 @@ static void ebt_load_match(const char *name) xs_init_match(m); if (m->x6_options != NULL) - opts = xtables_options_xfrm(opts, NULL, + opts = xtables_options_xfrm(xt_params->orig_opts, opts, m->x6_options, &m->option_offset); else if (m->extra_opts != NULL) - opts = xtables_merge_options(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; @@ -332,11 +335,16 @@ static void ebt_load_watcher(const char *name) xs_init_target(watcher); if (watcher->x6_options != NULL) - opts = xtables_options_xfrm(opts, NULL, watcher->x6_options, + opts = xtables_options_xfrm(xt_params->orig_opts, opts, + watcher->x6_options, &watcher->option_offset); else if (watcher->extra_opts != NULL) - opts = xtables_merge_options(opts, NULL, watcher->extra_opts, + opts = xtables_merge_options(xt_params->orig_opts, opts, + watcher->extra_opts, &watcher->option_offset); + else + return; + if (opts == NULL) xtables_error(OTHER_PROBLEM, "Can't alloc memory"); xt_params->opts = opts; @@ -344,7 +352,6 @@ static void ebt_load_watcher(const char *name) static void ebt_load_match_extensions(void) { - xt_params->opts = ebt_original_options; ebt_load_match("802_3"); ebt_load_match("arp"); ebt_load_match("ip"); @@ -358,10 +365,6 @@ static void ebt_load_match_extensions(void) ebt_load_watcher("log"); ebt_load_watcher("nflog"); - - /* assign them back so do_parse() may - * reset opts to orig_opts upon each call */ - xt_params->orig_opts = xt_params->opts; } void ebt_add_match(struct xtables_match *m, @@ -531,7 +534,6 @@ int nft_init_eb(struct nft_handle *h, const char *pname) void nft_fini_eb(struct nft_handle *h) { - struct option *opts = xt_params->opts; struct xtables_match *match; struct xtables_target *target; @@ -542,8 +544,7 @@ void nft_fini_eb(struct nft_handle *h) free(target->t); } - if (opts != ebt_original_options) - free(opts); + free(xt_params->opts); nft_fini(h); xtables_fini(); |