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

Commit a51a2885 authored by Remi NGUYEN VAN's avatar Remi NGUYEN VAN Committed by Lorenzo Colitti
Browse files

Fix potential exceptions in FdEventReader users 

In particular,
 - Fix a possible NPE In IpNeighborMonitor, if a NUD_FAILED is received
   for some neighbor before all other neighbors have received
   RTM_NEWNEIGH.
 - Add try / catch in IpNeighborMonitor, DhcpPacketListener,
   ConnectivityPacketTracker in case some unexpected exception is thrown
   while processing packets.
 - Add Log.wtf logging in FdEventsReader in case any such exception is
   missed.

Bug: 152842850
Test: atest NetworkStackTests
Original-Change: https://android-review.googlesource.com/1274609
Merged-In: Ia03fec358e98b3f218220e70fc3265e14ca15f78
Change-Id: Ia03fec358e98b3f218220e70fc3265e14ca15f78
parent b91dc822

common/moduleutils/src/android/net/ip/NetlinkMonitor.java

+17 −13
Original line number Diff line number Diff line
@@ -106,6 +106,7 @@ public class NetlinkMonitor extends PacketReader { 
        byteBuffer.order(ByteOrder.nativeOrder());



        while (byteBuffer.remaining() > 0) {

            try {

                final int position = byteBuffer.position();

                final NetlinkMessage nlMsg = NetlinkMessage.parse(byteBuffer);

                if (nlMsg == null || nlMsg.getHeader() == null) {
@@ -120,6 +121,9 @@ public class NetlinkMonitor extends PacketReader { 
                }



                processNetlinkMessage(nlMsg, whenMs);

            } catch (Throwable e) {

                mLog.e("Error handling netlink message", e);

            }

        }

    }

common/moduleutils/src/android/net/util/FdEventsReader.java

+2 −0
Original line number Diff line number Diff line
@@ -24,6 +24,7 @@ import android.os.Looper; 
import android.os.MessageQueue;

import android.system.ErrnoException;

import android.system.OsConstants;

import android.util.Log;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;
@@ -244,6 +245,7 @@ public abstract class FdEventsReader<BufferType> { 
                handlePacket(mBuffer, bytesRead);

            } catch (Exception e) {

                logError("handlePacket error: ", e);

                Log.wtf(FdEventsReader.class.getSimpleName(), "Error handling packet: stopping", e);

                break;

            }

        }

src/android/net/dhcp/DhcpPacketListener.java

+3 −0
Original line number Diff line number Diff line
@@ -19,6 +19,7 @@ package android.net.dhcp; 
import android.net.util.FdEventsReader;

import android.os.Handler;

import android.system.Os;

import android.util.Log;



import androidx.annotation.NonNull;

import androidx.annotation.Nullable;
@@ -59,6 +60,8 @@ abstract class DhcpPacketListener extends FdEventsReader<DhcpPacketListener.Payl 
            onReceive(packet, recvbuf.mSrcAddr, recvbuf.mSrcPort);

        } catch (DhcpPacket.ParseException e) {

            logParseError(recvbuf.mBytes, length, e);

        } catch (Throwable e) {

            Log.wtf(DhcpPacketListener.class.getSimpleName(), "Error handling DHCP packet", e);

        }

    }

src/android/net/ip/ConnectivityPacketTracker.java

+8 −3
Original line number Diff line number Diff line
@@ -133,9 +133,14 @@ public class ConnectivityPacketTracker { 
                return;

            }



            final String summary = ConnectivityPacketSummary.summarize(

                    mInterface.macAddr, recvbuf, length);

            final String summary;

            try {

                summary = ConnectivityPacketSummary.summarize(mInterface.macAddr, recvbuf, length);

                if (summary == null) return;

            } catch (Exception e) {

                if (DBG) Log.d(mTag, "Error creating packet summary", e);

                return;

            }



            if (DBG) Log.d(mTag, summary);

            addLogEntry(summary + "\n[" + HexDump.toHexString(recvbuf, 0, length) + "]");

src/android/net/ip/IpReachabilityMonitor.java

+12 −3
Original line number Diff line number Diff line
@@ -341,7 +341,14 @@ public class IpReachabilityMonitor { 
            // TODO: Consider using NeighborEvent#isValid() here; it's more

            // strict but may interact badly if other entries are somehow in

            // NUD_INCOMPLETE (say, during network attach).

            if (entry.getValue().nudState != StructNdMsg.NUD_FAILED) continue;

            final NeighborEvent val = entry.getValue();



            // Find all the neighbors that have gone into FAILED state.

            // Ignore entries for which we have never received an event. If there are neighbors

            // that never respond to ARP/ND, the kernel will send several FAILED event, then

            // an INCOMPLETE event, and then more FAILED events. The INCOMPLETE event will

            // populate the map and the subsequent FAILED event will be processed.

            if (val == null || val.nudState != StructNdMsg.NUD_FAILED) continue;



            ip = entry.getKey();

            for (RouteInfo route : mLinkProperties.getRoutes()) {
@@ -378,10 +385,12 @@ public class IpReachabilityMonitor { 
                Log.d(TAG, "neighbour IPv4(v6): " + entry.getKey() + " neighbour state: "

                        + StructNdMsg.stringForNudState(entry.getValue().nudState));

            }

            if (entry.getValue().nudState != StructNdMsg.NUD_REACHABLE) return;

            final NeighborEvent val = entry.getValue();

            // If an entry is null, consider that probing for that neighbour has completed.

            if (val == null || val.nudState != StructNdMsg.NUD_REACHABLE) return;

        }



        // All neighbours in the watchlist are in REACHABLE state and connection is stable,

        // Probing for all neighbours in the watchlist is complete and the connection is stable,

        // restore NUD probe parameters to steadystate value. In the case where neighbours

        // are responsive, this code will run before the wakelock expires.

        setNeighbourParametersForSteadyState();