commit bd97cc8ebf6c10ed6cb750eb2576fab6f307e7d1
author Josh Gao <jmgao@google.com> Tue Jun 16 15:58:11 2020 -0700
committer Lorenzo Colitti <lorenzo@google.com> Fri Aug 07 03:22:38 2020 +0000
tree c9b29fd8bd2021faf3b1791454c53923db8aa7e4
parent ed78646808b13d25e4adce2bf380c2241e697f63

Fix fd leak in KeepaliveTracker.

The semantics of FileDescriptor in AIDL are that the callee must close
the file descriptor it receives manually.

Bug: http://b/157789860
Test: treehugger
Change-Id: Ice9fc9abe2959a84ad138a95c900dff676653665

services/core/java/com/android/server/ConnectivityService.java

diff --git a/services/core/java/com/android/server/ConnectivityService.java b/services/core/java/com/android/server/ConnectivityService.java
index e1bcb0c..81bbf68 100644
--- a/services/core/java/com/android/server/ConnectivityService.java
+++ b/services/core/java/com/android/server/ConnectivityService.java
@@ -220,6 +220,8 @@
 
 import com.google.android.collect.Lists;
 
+import libcore.io.IoUtils;
+
 import org.xmlpull.v1.XmlPullParser;
 import org.xmlpull.v1.XmlPullParserException;
 
@@ -7504,18 +7506,34 @@
     public void startNattKeepaliveWithFd(Network network, FileDescriptor fd, int resourceId,
             int intervalSeconds, ISocketKeepaliveCallback cb, String srcAddr,
             String dstAddr) {
-        mKeepaliveTracker.startNattKeepalive(
-                getNetworkAgentInfoForNetwork(network), fd, resourceId,
-                intervalSeconds, cb,
-                srcAddr, dstAddr, NattSocketKeepalive.NATT_PORT);
+        try {
+            mKeepaliveTracker.startNattKeepalive(
+                    getNetworkAgentInfoForNetwork(network), fd, resourceId,
+                    intervalSeconds, cb,
+                    srcAddr, dstAddr, NattSocketKeepalive.NATT_PORT);
+        } finally {
+            // FileDescriptors coming from AIDL calls must be manually closed to prevent leaks.
+            // startNattKeepalive calls Os.dup(fd) before returning, so we can close immediately.
+            if (fd != null && Binder.getCallingPid() != Process.myPid()) {
+                IoUtils.closeQuietly(fd);
+            }
+        }
     }
 
     @Override
     public void startTcpKeepalive(Network network, FileDescriptor fd, int intervalSeconds,
             ISocketKeepaliveCallback cb) {
-        enforceKeepalivePermission();
-        mKeepaliveTracker.startTcpKeepalive(
-                getNetworkAgentInfoForNetwork(network), fd, intervalSeconds, cb);
+        try {
+            enforceKeepalivePermission();
+            mKeepaliveTracker.startTcpKeepalive(
+                    getNetworkAgentInfoForNetwork(network), fd, intervalSeconds, cb);
+        } finally {
+            // FileDescriptors coming from AIDL calls must be manually closed to prevent leaks.
+            // startTcpKeepalive calls Os.dup(fd) before returning, so we can close immediately.
+            if (fd != null && Binder.getCallingPid() != Process.myPid()) {
+                IoUtils.closeQuietly(fd);
+            }
+        }
     }
 
     @Override