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

Commit 7dc8ef83 authored by Xiao Ma's avatar Xiao Ma
Browse files

Prevent the potential NPE when toggling interface quickly. 

Querying interface parameters in different states has race if
toggling interface very quickly, for example, remove the interface
before transition to ClearingIpAddressState and add it back soon
immediately, that will introduce the different interface parameters
value. Global member "mInterfaceParams" is null when query occurs
in the ClearingIpAddressState since the interface has been removed
while local variable "params" isn't, then NPE will be thrown upon
retrieving the interface index when attempting to restore MTU.

Bug: 162808916
Test: atest NetworkStackIntegrationTests --iterations 100
Change-Id: I346f917aa1ff09342940d3a9a0c468483c279cdb
parent e639bf3b

src/android/net/ip/IpClient.java

+5 −1
Original line number Diff line number Diff line
@@ -1583,7 +1583,11 @@ public class IpClient extends StateMachine { 
            return;

        }



        if (params.index != mInterfaceParams.index) {

        // Check whether "mInterfaceParams" is null or not to prevent the potential NPE

        // introduced if the interface was initially not found, but came back before this

        // method was called. See b/162808916 for more details. TODO: query the new interface

        // parameters by the interface index instead and check that the index has not changed.

        if (mInterfaceParams == null || params.index != mInterfaceParams.index) {

            Log.w(mTag, "interface: " + mInterfaceName + " has a different index: " + params.index);

            return;

        }

tests/integration/src/android/net/ip/IpClientIntegrationTest.java

+48 −0
Original line number Diff line number Diff line
@@ -282,6 +282,7 @@ public class IpClientIntegrationTest { 
        private DhcpClient mDhcpClient;

        private boolean mIsHostnameConfigurationEnabled;

        private String mHostname;

        private boolean mIsInterfaceRecovered;



        public void setDhcpLeaseCacheEnabled(final boolean enable) {

            mIsDhcpLeaseCacheEnabled = enable;
@@ -300,6 +301,23 @@ public class IpClientIntegrationTest { 
            mHostname = hostname;

        }



        // Enable this flag to simulate the interface has been added back after removing

        // on the provisioning start. However, the actual tap interface has been removed,

        // interface parameters query will get null when attempting to restore Interface

        // MTU. Create a new InterfaceParams instance and return instead just for interface

        // toggling test case.

        public void simulateInterfaceRecover() {

            mIsInterfaceRecovered = true;

        }



        @Override

        public InterfaceParams getInterfaceParams(String ifname) {

            return mIsInterfaceRecovered

                    ? new InterfaceParams(ifname, 1 /* index */,

                            MacAddress.fromString("00:11:22:33:44:55"))

                    : super.getInterfaceParams(ifname);

        }



        @Override

        public INetd getNetd(Context context) {

            return mNetd;
@@ -1232,6 +1250,36 @@ public class IpClientIntegrationTest { 
        assertEquals(NetworkInterface.getByName(mIfaceName).getMTU(), TEST_DEFAULT_MTU);

    }



    @Test

    public void testRestoreInitialInterfaceMtu_removeInterfaceAndAddback() throws Exception {

        doAnswer(invocation -> {

            final LinkProperties lp = invocation.getArgument(0);

            assertEquals(lp.getInterfaceName(), mIfaceName);

            assertEquals(0, lp.getLinkAddresses().size());

            assertEquals(0, lp.getDnsServers().size());



            mDependencies.simulateInterfaceRecover();

            return null;

        }).when(mCb).onProvisioningFailure(any());



        final ProvisioningConfiguration config = new ProvisioningConfiguration.Builder()

                .withoutIpReachabilityMonitor()

                .withoutIPv6()

                .build();



        // Intend to remove the tap interface and force IpClient throw provisioning failure

        // due to that interface is not found.

        removeTapInterface(mTapFd);

        assertNull(InterfaceParams.getByName(mIfaceName));



        mIpc.startProvisioning(config);

        verify(mCb, timeout(TEST_TIMEOUT_MS)).onProvisioningFailure(any());



        // Make sure everything queued by this test was processed (e.g. transition to StoppingState

        // from ClearingIpAddressState) and tearDown will check if IpClient exits normally or crash.

        HandlerUtils.waitForIdle(mIpc.getHandler(), TEST_TIMEOUT_MS);

    }



    private boolean isRouterSolicitation(final byte[] packetBytes) {

        ByteBuffer packet = ByteBuffer.wrap(packetBytes);

        return packet.getShort(ETHER_TYPE_OFFSET) == (short) ETH_P_IPV6