Hi Maciej,
On Tue, Aug 11, 2020 at 12:07:12PM -0700, Maciej Żenczykowski wrote:
> We've gotten reports of a persistent memory leak in long running
> ip{,6}tables-restore processes on Android R (where we merged in a
> newer iptables release).
>
> Memory usage has been seen to hit 100MB each.
Did you test current code? I fixed quite a bunch of memleaks recently.
Did you capture iptables-restore input so we could build a reproducer?
> As a reminder, android runs iptables-restore/ip6tables-restore as a
> long running child process of netd (to eliminate process startup
> overhead, which is not insignificant) and communicates to it over a
> pipe. We don't use 'nft' nor 'ebtables'.
>
> To quote/paraphrase a coworker: "I got a heap profile while playing
> around with the networking config (connect & disconnect from wifi,
> change hotspot params etc) which repros a leak. It looks like
> xtables_find_target can malloc, but we don't free the results."
>
> We believe (unconfirmed) it's likely due to this commit:
> commit 148131f20421046fea028e638581e938ec985783[log] [tgz]
> author Phil Sutter <phil@xxxxxx>Mon Feb 04 21:52:53 2019 +0100
> committer Florian Westphal <fw@xxxxxxxxx>Tue Feb 05 16:09:41 2019 +0100
> xtables: Fix for false-positive rule matching
>
> Is this known? Fixed? I don't really understand what the commit is
> trying to accomplish/fix. Could we just revert that commit (or the
> libxtables/xtables.c portion there of)? Or is it perhaps obvious
> where the free should be happening?
Not explicitly, but maybe the following commits are worth a try:
432a5ecfa7890bd3495bb1ab5e34c2258090133f
b199aca80da5741add50cce244492cc005230b66
Not sure if they apply to your code-base, though.
> (and is there a similar problem with matches? the target and match
> code seem equivalent wrt. to this clone behaviour, maybe there's a
> similar issue)
Not to my knowledge. FWIW, I enhanced iptables/tests/shell for leak
checking, all iptables command calls exiting 0 properly free all memory
now. Of course this is not guaranteed to catch problems in
iptables-restore not exiting but reiterating over the same tables again
and again.
BTW: The commit you refer to fixes a problem where iptables would
incorrectly match a rule as equal although it's not. So without it, if
you have two rules which only differ in target payload (e.g., value of
MARK target), a rule delete command will always drop the first one found
no matter the target payload you passed. If you only delete by rule
number, you shouldn't need it.
Cheers, Phil
