diff options
author | Chiachang Wang <chiachangwang@google.com> | 2019-11-28 13:57:54 +0800 |
---|---|---|
committer | Chiachang Wang <chiachangwang@google.com> | 2019-11-28 13:57:54 +0800 |
commit | 19f87a7f449ceeaa4285ff4a80a4760691c56aa8 (patch) | |
tree | 9e4691eff19e5f12bbfb3cc129a4d7f8b16f39b2 /src | |
parent | f6d6cd295cc43b9420f0d06658ae8d8f5469793a (diff) |
Catch possible buffer access exception
Returned message from kernel in cf device may not work
as intended and crash NetworkStack. Since NetworkStack stays
in the system process, the crash will break device and impact
users. Catch the exception to prevent break whole device.
Bug: 145275899
Bug: 142035706
Test: atest NetworkStackTests NetworkStackNextTests
Change-Id: I7b7ecacfc76c54748a4428f04c5422506d85b8d2
Diffstat (limited to 'src')
-rw-r--r-- | src/com/android/networkstack/netlink/TcpSocketTracker.java | 74 |
1 files changed, 42 insertions, 32 deletions
diff --git a/src/com/android/networkstack/netlink/TcpSocketTracker.java b/src/com/android/networkstack/netlink/TcpSocketTracker.java index 5c30c30..cd21c32 100644 --- a/src/com/android/networkstack/netlink/TcpSocketTracker.java +++ b/src/com/android/networkstack/netlink/TcpSocketTracker.java @@ -63,8 +63,10 @@ import com.android.internal.annotations.VisibleForTesting; import java.io.FileDescriptor; import java.io.InterruptedIOException; import java.net.SocketException; +import java.nio.BufferUnderflowException; import java.nio.ByteBuffer; import java.util.ArrayList; +import java.util.Base64; import java.util.List; /** @@ -172,39 +174,47 @@ public class TcpSocketTracker { // | struct nlmsghdr | struct rtmsg | struct rtattr| data | // +------------------+---------------+--------------+--------+ final ByteBuffer bytes = mDependencies.recvMesssage(fd); - - while (enoughBytesRemainForValidNlMsg(bytes)) { - final StructNlMsgHdr nlmsghdr = StructNlMsgHdr.parse(bytes); - if (nlmsghdr == null) { - Log.e(TAG, "Badly formatted data."); - break; - } - final int nlmsgLen = nlmsghdr.nlmsg_len; - log("pollSocketsInfo: nlmsghdr=" + nlmsghdr); - // End of the message. Stop parsing. - if (nlmsghdr.nlmsg_type == NLMSG_DONE) break; - - if (nlmsghdr.nlmsg_type != SOCK_DIAG_BY_FAMILY) { - Log.e(TAG, "Expect to get family " + family - + " SOCK_DIAG_BY_FAMILY message but get " + nlmsghdr.nlmsg_type); - break; - } - - if (isValidInetDiagMsgSize(nlmsgLen)) { - // Get the socket cookie value. Composed by two Integers value. - // Corresponds to inet_diag_sockid in - // <linux_src>/include/uapi/linux/inet_diag.h - bytes.position(bytes.position() + IDIAG_COOKIE_OFFSET); - // It's stored in native with 2 int. Parse it as long for convenience. - final long cookie = bytes.getLong(); - // Skip the rest part of StructInetDiagMsg. - bytes.position(bytes.position() - + StructInetDiagMsg.STRUCT_SIZE - IDIAG_COOKIE_OFFSET - Long.BYTES); - final SocketInfo info = parseSockInfo(bytes, family, nlmsgLen, time); - // Update TcpStats based on previous and current socket info. - stat.accumulate(calculateLatestPacketsStat(info, mSocketInfos.get(cookie))); - mSocketInfos.put(cookie, info); + try { + while (enoughBytesRemainForValidNlMsg(bytes)) { + final StructNlMsgHdr nlmsghdr = StructNlMsgHdr.parse(bytes); + if (nlmsghdr == null) { + Log.e(TAG, "Badly formatted data."); + break; + } + final int nlmsgLen = nlmsghdr.nlmsg_len; + log("pollSocketsInfo: nlmsghdr=" + nlmsghdr + ", limit=" + bytes.limit()); + // End of the message. Stop parsing. + if (nlmsghdr.nlmsg_type == NLMSG_DONE) break; + + if (nlmsghdr.nlmsg_type != SOCK_DIAG_BY_FAMILY) { + Log.e(TAG, "Expect to get family " + family + + " SOCK_DIAG_BY_FAMILY message but get " + + nlmsghdr.nlmsg_type); + break; + } + + if (isValidInetDiagMsgSize(nlmsgLen)) { + // Get the socket cookie value. Composed by two Integers value. + // Corresponds to inet_diag_sockid in + // <linux_src>/include/uapi/linux/inet_diag.h + bytes.position(bytes.position() + IDIAG_COOKIE_OFFSET); + // It's stored in native with 2 int. Parse it as long for convenience. + final long cookie = bytes.getLong(); + // Skip the rest part of StructInetDiagMsg. + bytes.position(bytes.position() + + StructInetDiagMsg.STRUCT_SIZE - IDIAG_COOKIE_OFFSET + - Long.BYTES); + final SocketInfo info = parseSockInfo(bytes, family, nlmsgLen, time); + // Update TcpStats based on previous and current socket info. + stat.accumulate( + calculateLatestPacketsStat(info, mSocketInfos.get(cookie))); + mSocketInfos.put(cookie, info); + } } + } catch (IllegalArgumentException | BufferUnderflowException e) { + Log.wtf(TAG, "Unexpected socket info parsing, " + e + ", family " + family + + " buffer:" + bytes + " " + + Base64.getEncoder().encodeToString(bytes.array())); } } // Calculate mLatestReceiveCount, mSentSinceLastRecv and mLatestPacketFailPercentage. |