Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit ddb53852 authored by Jeff Vander Stoep's avatar Jeff Vander Stoep Committed by Luca Stefani
Browse files

Fix FD leak in ConnectivityManager.getConnectionOwnerUid 

Add unit tests to verify that bug has been fixed.

Re-enable testGetConnectionOwnerUid() unit tests in presubmit. These
were disabled due to test flakiness caused by expected failures passing
as a result of other sockets on the system. This is fixed by checking
that failures do not have the UID of the calling process instead of
INVALID_UID since previously some Qualcomm telephony sockets were
causing lookup successes.

Test: atest InetDiagSocketTest#testGetConnectionOwnerUid
Test: ls -1 /proc/<pid of system_server>/fd | wca
Test: atest --generate-new-metrics 200 InetDiagSocketTest#testGetConnectionOwnerUid
   To verify flakes have been cleaned up.
Bug: 141603906
Bug: 141459241
Change-Id: Ib76674f10e4bd24952c557bac7b9c65fba42fdb2
parent 67b1e0b8

services/net/java/android/net/netlink/InetDiagMessage.java

+16 −11
Original line number Diff line number Diff line
@@ -16,26 +16,23 @@ 


package android.net.netlink;



import static android.os.Process.INVALID_UID;

import static android.net.netlink.NetlinkConstants.SOCK_DIAG_BY_FAMILY;

import static android.net.netlink.NetlinkSocket.DEFAULT_RECV_BUFSIZE;

import static android.net.netlink.StructNlMsgHdr.NLM_F_DUMP;

import static android.net.netlink.StructNlMsgHdr.NLM_F_REQUEST;

import static android.os.Process.INVALID_UID;

import static android.system.OsConstants.AF_INET;

import static android.system.OsConstants.AF_INET6;

import static android.system.OsConstants.IPPROTO_UDP;

import static android.system.OsConstants.NETLINK_INET_DIAG;



import android.os.Build;

import android.os.Process;

import android.net.util.SocketUtils;

import android.system.ErrnoException;

import android.util.Log;



import java.io.FileDescriptor;

import java.io.IOException;

import java.io.InterruptedIOException;

import java.net.DatagramSocket;

import java.net.DatagramSocket;

import java.net.InetAddress;

import java.net.Inet4Address;

import java.net.Inet6Address;

import java.net.InetSocketAddress;
@@ -163,17 +160,25 @@ public class InetDiagMessage extends NetlinkMessage { 
     */

    public static int getConnectionOwnerUid(int protocol, InetSocketAddress local,

                                            InetSocketAddress remote) {

        int uid = INVALID_UID;

        FileDescriptor fd = null;

        try {

            final FileDescriptor fd = NetlinkSocket.forProto(NETLINK_INET_DIAG);

            fd = NetlinkSocket.forProto(NETLINK_INET_DIAG);

            NetlinkSocket.connectToKernel(fd);



            return lookupUid(protocol, local, remote, fd);



            uid = lookupUid(protocol, local, remote, fd);

        } catch (ErrnoException | SocketException | IllegalArgumentException

                | InterruptedIOException e) {

            Log.e(TAG, e.toString());

        } finally {

            if (fd != null) {

                try {

                    SocketUtils.closeSocket(fd);

                } catch (IOException e) {

                    Log.e(TAG, e.toString());

                }

        return INVALID_UID;

            }

        }

        return uid;

    }



    @Override

tests/net/java/android/net/netlink/InetDiagSocketTest.java

+21 −14
Original line number Diff line number Diff line
@@ -18,7 +18,6 @@ package android.net.netlink; 


import static android.net.netlink.StructNlMsgHdr.NLM_F_DUMP;

import static android.net.netlink.StructNlMsgHdr.NLM_F_REQUEST;

import static android.os.Process.INVALID_UID;

import static android.system.OsConstants.AF_INET;

import static android.system.OsConstants.AF_INET6;

import static android.system.OsConstants.IPPROTO_TCP;
@@ -28,6 +27,7 @@ import static android.system.OsConstants.SOCK_STREAM; 


import static org.junit.Assert.assertArrayEquals;

import static org.junit.Assert.assertEquals;

import static org.junit.Assert.assertNotEquals;

import static org.junit.Assert.assertNotNull;

import static org.junit.Assert.assertTrue;
@@ -45,7 +45,6 @@ import androidx.test.runner.AndroidJUnit4; 
import libcore.util.HexEncoding;



import org.junit.Before;

import org.junit.Ignore;

import org.junit.Test;

import org.junit.runner.RunWith;
@@ -152,9 +151,13 @@ public class InetDiagSocketTest { 


    private void checkConnectionOwnerUid(int protocol, InetSocketAddress local,

                                         InetSocketAddress remote, boolean expectSuccess) {

        final int expectedUid = expectSuccess ? Process.myUid() : INVALID_UID;

        final int uid = mCm.getConnectionOwnerUid(protocol, local, remote);

        assertEquals(expectedUid, uid);



        if (expectSuccess) {

            assertEquals(Process.myUid(), uid);

        } else {

            assertNotEquals(Process.myUid(), uid);

        }

    }



    private int findLikelyFreeUdpPort(UdpConnection conn) throws Exception {
@@ -165,11 +168,11 @@ public class InetDiagSocketTest { 
        return localPort;

    }



    public void checkGetConnectionOwnerUid(String to, String from) throws Exception {

    /**

         * For TCP connections, create a test connection and verify that this

     * Create a test connection for UDP and TCP sockets and verify that this

     * {protocol, local, remote} socket result in receiving a valid UID.

     */

    public void checkGetConnectionOwnerUid(String to, String from) throws Exception {

        TcpConnection tcp = new TcpConnection(to, from);

        checkConnectionOwnerUid(tcp.protocol, tcp.local, tcp.remote, true);

        checkConnectionOwnerUid(IPPROTO_UDP, tcp.local, tcp.remote, false);
@@ -177,20 +180,14 @@ public class InetDiagSocketTest { 
        checkConnectionOwnerUid(tcp.protocol, tcp.local, new InetSocketAddress(0), false);

        tcp.close();



        /**

         * For UDP connections, either a complete match {protocol, local, remote} or a

         * partial match {protocol, local} should return a valid UID.

         */

        UdpConnection udp = new UdpConnection(to,from);

        checkConnectionOwnerUid(udp.protocol, udp.local, udp.remote, true);

        checkConnectionOwnerUid(udp.protocol, udp.local, new InetSocketAddress(0), true);

        checkConnectionOwnerUid(IPPROTO_TCP, udp.local, udp.remote, false);

        checkConnectionOwnerUid(udp.protocol, new InetSocketAddress(findLikelyFreeUdpPort(udp)),

                udp.remote, false);

        udp.close();

    }



    @Ignore

    @Test

    public void testGetConnectionOwnerUid() throws Exception {

        checkGetConnectionOwnerUid("::", null);
@@ -203,6 +200,16 @@ public class InetDiagSocketTest { 
        checkGetConnectionOwnerUid("::1", "::1");

    }



    /* Verify fix for b/141603906 */

    @Test

    public void testB141603906() throws Exception {

        final InetSocketAddress src = new InetSocketAddress(0);

        final InetSocketAddress dst = new InetSocketAddress(0);

        for (int i = 1; i <= 100000; i++) {

            mCm.getConnectionOwnerUid(IPPROTO_TCP, src, dst);

        }

    }



    // Hexadecimal representation of InetDiagReqV2 request.

    private static final String INET_DIAG_REQ_V2_UDP_INET4_HEX =

            // struct nlmsghdr