ANDROID: do not leak memory in iptables-restore
This code was introduced in:
commit 148131f20421046fea028e638581e938ec985783
Author: Phil Sutter <phil@nwl.cc>
Date: Mon Feb 4 21:52:53 2019 +0100
xtables: Fix for false-positive rule matching
When comparing two rules with non-standard targets, differences in
targets' payloads wasn't respected.
The cause is a rather hideous one: Unlike xtables_find_match(),
xtables_find_target() did not care whether the found target was already
in use or not, so the same target instance was assigned to both rules
and therefore payload comparison happened over the same memory location.
With legacy iptables it is not possible to reuse a target: The only case
where two rules (i.e., iptables_command_state instances) could exist at
the same time is when comparing rules, but that's handled using libiptc.
The above change clashes with ebtables-nft's reuse of target objects:
While input parsing still just assigns the object from xtables_targets
list, rule conversion from nftnl to iptables_command_state allocates new
data. To fix this, make ebtables-nft input parsing use the common
command_jump() routine instead of its own simplified copy. In turn, this
also eliminates the ebtables-nft-specific variants of parse_target(),
though with a slight change of behaviour: Names of user-defined chains
are no longer allowed to contain up to 31 but merely 28 characters.
Signed-off-by: Phil Sutter <phil@nwl.cc>
Signed-off-by: Florian Westphal <fw@strlen.de>
I'm not utterly convinced this doesn't reintroduce a bug wrt. matching
more complex targets... but oh well, we didn't need it in Q presumably
we still don't need it.
Testable via:
Running from one terminal in one adb session:
#!/bin/sh
cmds() {
echo '*mangle'
echo '-A INPUT -s 1.1.1.1 -j DROP'
echo 'COMMIT'
echo '#PING'
echo '*mangle'
echo '-D INPUT -s 1.1.1.1 -j DROP'
echo 'COMMIT'
echo '#PING'
echo -n '.' 1>&2
}
while true; do cmds; done | iptables-restore --noflush -w -v
And then in another terminal:
$ adbz shell ps | egrep iptables-restore && adbz shell
(the second match is likely the one you started up above, the first is probably netd's)
vsoc_x86:/ # while true; do cat /proc/${THE_PID_FROM_ABOVE}/status | egrep VmData; sleep 1; done
... does not grow (seems to stay under 10MiB)
Test: atest
Bug: 162925719
Bug: 168688680
Signed-off-by: Maciej Żenczykowski <maze@google.com>
Change-Id: Idcd81084e05acadf33ba885084d67e26082dc9b2
Merged-In: Idcd81084e05acadf33ba885084d67e26082dc9b2
(cherry picked from commit db325961118ca9c1b2179698d4c69b568b04470c)
diff --git a/libxtables/xtables.c b/libxtables/xtables.c
index 895f698..9c5827e 100644
--- a/libxtables/xtables.c
+++ b/libxtables/xtables.c
@@ -757,6 +757,7 @@
for (ptr = xtables_targets; ptr; ptr = ptr->next) {
if (extension_cmp(name, ptr->name, ptr->family)) {
+#if 0 /* Code block below causes memory leak. (Bugs 162925719 and 168688680) */
struct xtables_target *clone;
/* First target of this type: */
@@ -772,6 +773,7 @@
clone->next = clone;
ptr = clone;
+#endif
break;
}
}
