commit 555472a186db381def14d2f27c8175d93418ff61
author Lorenzo Colitti <lorenzo@google.com> Tue Feb 09 23:07:17 2021 +0900
committer Lorenzo Colitti <lorenzo@google.com> Tue Feb 09 23:30:25 2021 +0900
tree dc14351f51ff91bbeadb478f8c88b70cfd9c3e38
parent 4431655a4d52c352ef7ff6606d7f129588116a6d

Don't leak map fds in BpfCoordinator#dump.

Currently, the dump methods open BpfMap objects and never close
them. This leaks filedescriptors, and if dump is called often
enough, will crash the networkstack process.

Fix this by using try-with-resources statements that
automatically close the map when exiting the try block.

Change the signature of BpfMap#close from "throws Exception" to
"throws ErrnoException" since it does not throw any other type
of checked exceptions.

Test: "lsof | grep network_st" while running "dumpsys tethering bpf" in a loop
Change-Id: I66c407454c2715bf41bf3a2e81bd582f9ea5a905

Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java

diff --git a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
index b17bfcf..4ba5130 100644
--- a/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
+++ b/Tethering/src/com/android/networkstack/tethering/BpfCoordinator.java
@@ -766,17 +766,16 @@
     }
 
     private void dumpIpv6UpstreamRules(IndentingPrintWriter pw) {
-        final BpfMap<TetherUpstream6Key, Tether6Value> ipv6UpstreamMap = mDeps.getBpfUpstream6Map();
-        if (ipv6UpstreamMap == null) {
-            pw.println("No IPv6 upstream");
-            return;
-        }
-        try {
-            if (ipv6UpstreamMap.isEmpty()) {
+        try (BpfMap<TetherUpstream6Key, Tether6Value> map = mDeps.getBpfUpstream6Map()) {
+            if (map == null) {
+                pw.println("No IPv6 upstream");
+                return;
+            }
+            if (map.isEmpty()) {
                 pw.println("No IPv6 upstream rules");
                 return;
             }
-            ipv6UpstreamMap.forEach((k, v) -> pw.println(ipv6UpstreamRuletoString(k, v)));
+            map.forEach((k, v) -> pw.println(ipv6UpstreamRuletoString(k, v)));
         } catch (ErrnoException e) {
             pw.println("Error dumping IPv4 map: " + e);
         }
@@ -797,19 +796,18 @@
     }
 
     private void dumpIpv4ForwardingRules(IndentingPrintWriter pw) {
-        final BpfMap<Tether4Key, Tether4Value> ipv4UpstreamMap = mDeps.getBpfUpstream4Map();
-        if (ipv4UpstreamMap == null) {
-            pw.println("No IPv4 support");
-            return;
-        }
-        try {
-            if (ipv4UpstreamMap.isEmpty()) {
+        try (BpfMap<Tether4Key, Tether4Value> map = mDeps.getBpfUpstream4Map()) {
+            if (map == null) {
+                pw.println("No IPv4 support");
+                return;
+            }
+            if (map.isEmpty()) {
                 pw.println("No IPv4 rules");
                 return;
             }
             pw.println("[IPv4]: iif(iface) oif(iface) src nat dst");
             pw.increaseIndent();
-            ipv4UpstreamMap.forEach((k, v) -> pw.println(ipv4RuleToString(k, v)));
+            map.forEach((k, v) -> pw.println(ipv4RuleToString(k, v)));
         } catch (ErrnoException e) {
             pw.println("Error dumping IPv4 map: " + e);
         }

Tethering/src/com/android/networkstack/tethering/BpfMap.java

diff --git a/Tethering/src/com/android/networkstack/tethering/BpfMap.java b/Tethering/src/com/android/networkstack/tethering/BpfMap.java
index 89caa8a..bc01dbd 100644
--- a/Tethering/src/com/android/networkstack/tethering/BpfMap.java
+++ b/Tethering/src/com/android/networkstack/tethering/BpfMap.java
@@ -218,7 +218,7 @@
     }
 
     @Override
-    public void close() throws Exception {
+    public void close() throws ErrnoException {
         closeMap(mMapFd);
     }