commit 321b22ceea5e79e5e8d7d990c9618afaa7f87c5c
author Hugo Benichi <hugobenichi@google.com> Tue Jan 30 11:37:27 2018 +0900
committer Hugo Benichi <hugobenichi@google.com> Tue Jan 30 16:05:18 2018 +0900
tree 8848536a099e11640b6aee5f3adefc475d62555f
parent dae2ecf45c5e34fc07b3a91b6fe2b378bb9a4441

TcpSocketMonitor code hardening

This patch adds a couple of additional guards to make TcpSocketMonitor
more robust:
  - TcpSocketMonitor::updateSocketStats() is not called if the Netlink
  message type does not match SOCK_DIAG_BY_FAMILY
  - the tcpinfo_get macro now correctly takes into account the length of
  the field that is accessed when comparing to the number of bytes
  available for struct tcp_info.

In additional, tcpi_data_segs_out which is not available on 4.4 kernels
is replaced by tcpi_segs_out.

Bug: 64147860, 72512637
Test: manual tests
Change-Id: I898fc07362788b7991594d303665a88f57bf1b35

server/TcpSocketMonitor.cpp

diff --git a/server/TcpSocketMonitor.cpp b/server/TcpSocketMonitor.cpp
index ce8df97..358e4c0 100644
--- a/server/TcpSocketMonitor.cpp
+++ b/server/TcpSocketMonitor.cpp
@@ -66,8 +66,9 @@
 
 // Helper macro for reading fields into struct tcp_info and handling different struct tcp_info
 // versions in the kernel.
-#define tcpinfo_get(ptr, fld, len, zero) \
-    (((ptr) != nullptr && offsetof(struct tcp_info, fld) < len) ? (ptr)->fld : zero)
+#define TCPINFO_GET(ptr, fld, len, zero) \
+        (((ptr) != nullptr && (offsetof(struct tcp_info, fld) + sizeof((ptr)->fld)) < len) ? \
+        (ptr)->fld : zero)
 
 static void tcpInfoPrint(DumpWriter &dw, Fwmark mark, const struct inet_diag_msg *sockinfo,
                          const struct tcp_info *tcpinfo, uint32_t tcpinfoLen) {
@@ -87,9 +88,9 @@
             ntohs(sockinfo->id.idiag_sport),
             ntohs(sockinfo->id.idiag_dport),
             getTcpStateName(sockinfo->idiag_state), sockinfo->idiag_state,
-            tcpinfo_get(tcpinfo, tcpi_rtt, tcpinfoLen, 0) / 1000.0,
-            tcpinfo_get(tcpinfo, tcpi_data_segs_out, tcpinfoLen, 0),
-            tcpinfo_get(tcpinfo, tcpi_lost, tcpinfoLen, 0));
+            TCPINFO_GET(tcpinfo, tcpi_rtt, tcpinfoLen, 0) / 1000.0,
+            TCPINFO_GET(tcpinfo, tcpi_segs_out, tcpinfoLen, 0),
+            TCPINFO_GET(tcpinfo, tcpi_lost, tcpinfoLen, 0));
 }
 
 const String16 TcpSocketMonitor::DUMP_KEYWORD = String16("tcp_socket_info");
@@ -275,12 +276,12 @@
                                          const struct inet_diag_msg *sockinfo,
                                          const struct tcp_info *tcpinfo,
                                          uint32_t tcpinfoLen) NO_THREAD_SAFETY_ANALYSIS {
-    int32_t lastAck = tcpinfo_get(tcpinfo, tcpi_last_ack_recv, tcpinfoLen, 0);
-    int32_t lastSent = tcpinfo_get(tcpinfo, tcpi_last_data_sent, tcpinfoLen, 0);
+    int32_t lastAck = TCPINFO_GET(tcpinfo, tcpi_last_ack_recv, tcpinfoLen, 0);
+    int32_t lastSent = TCPINFO_GET(tcpinfo, tcpi_last_data_sent, tcpinfoLen, 0);
     TcpStats diff = {
-        .sent = tcpinfo_get(tcpinfo, tcpi_data_segs_out, tcpinfoLen, 0),
-        .lost = tcpinfo_get(tcpinfo, tcpi_lost, tcpinfoLen, 0),
-        .rttUs = tcpinfo_get(tcpinfo, tcpi_rtt, tcpinfoLen, 0),
+        .sent = TCPINFO_GET(tcpinfo, tcpi_segs_out, tcpinfoLen, 0),
+        .lost = TCPINFO_GET(tcpinfo, tcpi_lost, tcpinfoLen, 0),
+        .rttUs = TCPINFO_GET(tcpinfo, tcpi_rtt, tcpinfoLen, 0),
         .sentAckDiffMs = lastAck - lastSent,
         .nSockets = 1,
     };