shill: add unit tests for OnRawNlMessageReceived
Add unit tests that verify that NetlinkManager handles malformed
netlink messages in a reasonable way. This led me to revise the
checks in OnRawNlMessageReceived...
We now validate that we have read enough bytes for an entire nlmsghdr,
instead of just the length field of the header. This eliminates the
possibility of a bad memory access when OnNlMessageReceived reads
the sequence number field of the nlmsghdr.
While there:
- Have the NetlinkManagerTest fixture check the nullity of
netlink_manager_ before dereferencing it.
- Fully reset the NetlinkManager singleton when the NetlinkManagerTest
fixture is destroyed. This ensures that each test finds the fixture
in the same initial state.
- Change netlink_socket_ from an inline member to a raw pointer, so
that we can pass ownership to NetlinkManager (but also retain a reference
to it).
BUG=chromium:313000
TEST=USE="asan clang" FEATURES="test" P2_TEST_FILTER="shill::*" emerge-peppy platform2
Change-Id: Ibdbe5574badf6963ade22dec546ade040c27686c
Reviewed-on: https://chromium-review.googlesource.com/175855
Reviewed-by: mukesh agrawal <quiche@chromium.org>
Commit-Queue: mukesh agrawal <quiche@chromium.org>
Tested-by: mukesh agrawal <quiche@chromium.org>
diff --git a/netlink_manager.cc b/netlink_manager.cc
index 9459c2b..afcc4b9 100644
--- a/netlink_manager.cc
+++ b/netlink_manager.cc
@@ -513,15 +513,8 @@
unsigned char *end = buf + data->len;
while (buf < end) {
nlmsghdr *msg = reinterpret_cast<nlmsghdr *>(buf);
- // Discard the message if there're not enough bytes to a) tell the code how
- // much space is in the message (i.e., to access nlmsg_len) or b) to hold
- // the entire message. The odd calculation is there to keep the code from
- // potentially calculating an illegal address (causes a segfault on some
- // architectures).
size_t bytes_left = end - buf;
- if (((bytes_left < (offsetof(nlmsghdr, nlmsg_len) +
- sizeof(msg->nlmsg_len))) ||
- (bytes_left < msg->nlmsg_len))) {
+ if (bytes_left < sizeof(nlmsghdr) || bytes_left < msg->nlmsg_len) {
LOG(ERROR) << "Discarding incomplete message.";
return;
}