When parsing a netlink message, always advance to the end.
NetlinkEvent#parse relies on message-specific classes to parse
the payload of the message. If these classes do not consume the
entire message, parse does not advance to the end of the
message. This means that any code that loops over a ByteBuffer
parsing messages one at a time (e.g., NetlinkMonitor) will think
that there is still a message left to parse, and will attempt
to parse the remainder of the message as a new netlink message.
This is obviously incorrect for any buffer that contains more
than one netlink message.
Bug: 163492391
Test: existing unit tests
Change-Id: Ifdb9748068857f75ca00c58f0c40f19809aae5c9
diff --git a/staticlibs/device/com/android/net/module/util/netlink/NetlinkMessage.java b/staticlibs/device/com/android/net/module/util/netlink/NetlinkMessage.java
index a216752..9e1e26e 100644
--- a/staticlibs/device/com/android/net/module/util/netlink/NetlinkMessage.java
+++ b/staticlibs/device/com/android/net/module/util/netlink/NetlinkMessage.java
@@ -51,8 +51,8 @@
return null;
}
- int payloadLength = NetlinkConstants.alignedLengthOf(nlmsghdr.nlmsg_len);
- payloadLength -= StructNlMsgHdr.STRUCT_SIZE;
+ final int messageLength = NetlinkConstants.alignedLengthOf(nlmsghdr.nlmsg_len);
+ final int payloadLength = messageLength - StructNlMsgHdr.STRUCT_SIZE;
if (payloadLength < 0 || payloadLength > byteBuffer.remaining()) {
// Malformed message or runt buffer. Pretend the buffer was consumed.
byteBuffer.position(byteBuffer.limit());
@@ -68,15 +68,22 @@
// Netlink family messages. The netlink family is required. Note that the reason for using
// if-statement is that switch-case can't be used because the OsConstants.NETLINK_* are
// not constant.
+ final NetlinkMessage parsed;
if (nlFamily == OsConstants.NETLINK_ROUTE) {
- return parseRtMessage(nlmsghdr, byteBuffer);
+ parsed = parseRtMessage(nlmsghdr, byteBuffer);
} else if (nlFamily == OsConstants.NETLINK_INET_DIAG) {
- return parseInetDiagMessage(nlmsghdr, byteBuffer);
+ parsed = parseInetDiagMessage(nlmsghdr, byteBuffer);
} else if (nlFamily == OsConstants.NETLINK_NETFILTER) {
- return parseNfMessage(nlmsghdr, byteBuffer);
+ parsed = parseNfMessage(nlmsghdr, byteBuffer);
+ } else {
+ parsed = null;
}
- return null;
+ // Advance to the end of the message, regardless of whether the parsing code consumed
+ // all of it or not.
+ byteBuffer.position(startPosition + messageLength);
+
+ return parsed;
}
@NonNull
diff --git a/staticlibs/tests/unit/src/com/android/net/module/util/netlink/RtNetlinkRouteMessageTest.java b/staticlibs/tests/unit/src/com/android/net/module/util/netlink/RtNetlinkRouteMessageTest.java
index 93b9894..55cfd50 100644
--- a/staticlibs/tests/unit/src/com/android/net/module/util/netlink/RtNetlinkRouteMessageTest.java
+++ b/staticlibs/tests/unit/src/com/android/net/module/util/netlink/RtNetlinkRouteMessageTest.java
@@ -195,11 +195,8 @@
// Try to parse the RTM_NEWADDR message.
msg = NetlinkMessage.parse(byteBuffer, NETLINK_ROUTE);
- // The current parse code does not advance to the end of the message if the
- // entire message wasn't consumed, then the remaining attributes in the
- // RTM_NEWROUTE message will be considered as another new rtm netlink message,
- // that will return null.
- assertNull(msg);
+ assertNotNull(msg);
+ assertTrue(msg instanceof RtNetlinkAddressMessage);
}
@Test