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

Commit 9890fcdc authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN
Browse files

Get link state with same lock as LinkProperties 

The previous IpClientLinkObserver code could in theory call
mCallback.update with a linkState that came from another update; for
example:
 1. onInterfaceAddressUpdated
   - mLinkProperties.addLinkAddress
 2. onInterfaceLinkStateChanged
   - setInterfaceLinkState(false)
 1. mCallback.update(false) <- should be true ?
 2. mCallback.update(false)

In practice this would not happen because the onFoo methods are all
called in order (same binder token from netd), but IpClientLinkObserver
should not need to make such assumptions.

Bug: 151796056
Test: atest NetworkStackIntegrationTests (see also test-only change)
Change-Id: I60f5a319519069070eb5a07643686bf5ec937665
parent 750fe188

src/android/net/ip/IpClientLinkObserver.java

+32 −16
Original line number Original line Diff line number Diff line
@@ -155,8 +155,12 @@ public class IpClientLinkObserver implements NetworkObserver { 
            // now empty. Note that from the moment that the interface is removed, any further

            // now empty. Note that from the moment that the interface is removed, any further

            // interface-specific messages (e.g., RTM_DELADDR) will not reach us, because the netd

            // interface-specific messages (e.g., RTM_DELADDR) will not reach us, because the netd

            // code that parses them will not be able to resolve the ifindex to an interface name.

            // code that parses them will not be able to resolve the ifindex to an interface name.

            final boolean linkState;

            synchronized (this) {

                clearLinkProperties();

                clearLinkProperties();

            mCallback.update(getInterfaceLinkState());

                linkState = getInterfaceLinkStateLocked();

            }

            mCallback.update(linkState);

        }

        }

    }

    }
@@ -164,7 +168,9 @@ public class IpClientLinkObserver implements NetworkObserver { 
    public void onInterfaceLinkStateChanged(String iface, boolean state) {

    public void onInterfaceLinkStateChanged(String iface, boolean state) {

        if (mInterfaceName.equals(iface)) {

        if (mInterfaceName.equals(iface)) {

            maybeLog("interfaceLinkStateChanged", iface + (state ? " up" : " down"));

            maybeLog("interfaceLinkStateChanged", iface + (state ? " up" : " down"));

            setInterfaceLinkState(state);

            synchronized (this) {

                setInterfaceLinkStateLocked(state);

            }

        }

        }

    }

    }
@@ -172,12 +178,14 @@ public class IpClientLinkObserver implements NetworkObserver { 
    public void onInterfaceAddressUpdated(LinkAddress address, String iface) {

    public void onInterfaceAddressUpdated(LinkAddress address, String iface) {

        if (mInterfaceName.equals(iface)) {

        if (mInterfaceName.equals(iface)) {

            maybeLog("addressUpdated", iface, address);

            maybeLog("addressUpdated", iface, address);

            boolean changed;

            final boolean changed;

            final boolean linkState;

            synchronized (this) {

            synchronized (this) {

                changed = mLinkProperties.addLinkAddress(address);

                changed = mLinkProperties.addLinkAddress(address);

                linkState = getInterfaceLinkStateLocked();

            }

            }

            if (changed) {

            if (changed) {

                mCallback.update(getInterfaceLinkState());

                mCallback.update(linkState);

            }

            }

        }

        }

    }

    }
@@ -186,12 +194,14 @@ public class IpClientLinkObserver implements NetworkObserver { 
    public void onInterfaceAddressRemoved(LinkAddress address, String iface) {

    public void onInterfaceAddressRemoved(LinkAddress address, String iface) {

        if (mInterfaceName.equals(iface)) {

        if (mInterfaceName.equals(iface)) {

            maybeLog("addressRemoved", iface, address);

            maybeLog("addressRemoved", iface, address);

            boolean changed;

            final boolean changed;

            final boolean linkState;

            synchronized (this) {

            synchronized (this) {

                changed = mLinkProperties.removeLinkAddress(address);

                changed = mLinkProperties.removeLinkAddress(address);

                linkState = getInterfaceLinkStateLocked();

            }

            }

            if (changed) {

            if (changed) {

                mCallback.update(getInterfaceLinkState());

                mCallback.update(linkState);

            }

            }

        }

        }

    }

    }
@@ -200,12 +210,14 @@ public class IpClientLinkObserver implements NetworkObserver { 
    public void onRouteUpdated(RouteInfo route) {

    public void onRouteUpdated(RouteInfo route) {

        if (mInterfaceName.equals(route.getInterface())) {

        if (mInterfaceName.equals(route.getInterface())) {

            maybeLog("routeUpdated", route);

            maybeLog("routeUpdated", route);

            boolean changed;

            final boolean changed;

            final boolean linkState;

            synchronized (this) {

            synchronized (this) {

                changed = mLinkProperties.addRoute(route);

                changed = mLinkProperties.addRoute(route);

                linkState = getInterfaceLinkStateLocked();

            }

            }

            if (changed) {

            if (changed) {

                mCallback.update(getInterfaceLinkState());

                mCallback.update(linkState);

            }

            }

        }

        }

    }

    }
@@ -214,12 +226,14 @@ public class IpClientLinkObserver implements NetworkObserver { 
    public void onRouteRemoved(RouteInfo route) {

    public void onRouteRemoved(RouteInfo route) {

        if (mInterfaceName.equals(route.getInterface())) {

        if (mInterfaceName.equals(route.getInterface())) {

            maybeLog("routeRemoved", route);

            maybeLog("routeRemoved", route);

            boolean changed;

            final boolean changed;

            final boolean linkState;

            synchronized (this) {

            synchronized (this) {

                changed = mLinkProperties.removeRoute(route);

                changed = mLinkProperties.removeRoute(route);

                linkState = getInterfaceLinkStateLocked();

            }

            }

            if (changed) {

            if (changed) {

                mCallback.update(getInterfaceLinkState());

                mCallback.update(linkState);

            }

            }

        }

        }

    }

    }
@@ -228,12 +242,14 @@ public class IpClientLinkObserver implements NetworkObserver { 
    public void onInterfaceDnsServerInfo(String iface, long lifetime, String[] addresses) {

    public void onInterfaceDnsServerInfo(String iface, long lifetime, String[] addresses) {

        if (mInterfaceName.equals(iface)) {

        if (mInterfaceName.equals(iface)) {

            maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses));

            maybeLog("interfaceDnsServerInfo", Arrays.toString(addresses));

            boolean changed = mDnsServerRepository.addServers(lifetime, addresses);

            final boolean changed = mDnsServerRepository.addServers(lifetime, addresses);

            final boolean linkState;

            if (changed) {

            if (changed) {

                synchronized (this) {

                synchronized (this) {

                    mDnsServerRepository.setDnsServersOn(mLinkProperties);

                    mDnsServerRepository.setDnsServersOn(mLinkProperties);

                    linkState = getInterfaceLinkStateLocked();

                }

                }

                mCallback.update(getInterfaceLinkState());

                mCallback.update(linkState);

            }

            }

        }

        }

    }

    }
@@ -258,11 +274,11 @@ public class IpClientLinkObserver implements NetworkObserver { 
        mLinkProperties.setInterfaceName(mInterfaceName);

        mLinkProperties.setInterfaceName(mInterfaceName);

    }

    }





    private synchronized boolean getInterfaceLinkState() {

    private boolean getInterfaceLinkStateLocked() {

        return mInterfaceLinkState;

        return mInterfaceLinkState;

    }

    }





    private synchronized void setInterfaceLinkState(boolean state) {

    private void setInterfaceLinkStateLocked(boolean state) {

        mInterfaceLinkState = state;

        mInterfaceLinkState = state;

    }

    }
@@ -378,7 +394,7 @@ public class IpClientLinkObserver implements NetworkObserver { 
                cancelPref64Alarm();

                cancelPref64Alarm();

            }

            }





            mCallback.update(getInterfaceLinkState());

            mCallback.update(getInterfaceLinkStateLocked());

        }

        }





        private void processPref64Option(StructNdOptPref64 opt, final long now) {

        private void processPref64Option(StructNdOptPref64 opt, final long now) {