tree 750fec3cf497e2b94f764c590074342282c34c76
parent 8e8de490462cef4470f63c448a19ba314bf31690
author Maciej Żenczykowski <maze@google.com> 1600338790 +0000
committer Maciej Zenczykowski <maze@google.com> 1600339793 +0000

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