commit 1782aea2babfa6fd1963bed907d8e31ffcb29e67
author Ken Chen <cken@google.com> Fri Feb 21 16:30:47 2020 +0800
committer Ken Chen <cken@google.com> Sat Feb 29 09:28:31 2020 +0800
tree 9c05a0c8e573c39fb5e4e0043ea3c70d0bf8a52f
parent fc1da892c7bbdf8786c215dc5badada3f25fce30

Pass connectInfo only if ro.vendor.redirect_socket_calls is set

There is a dependency between libnetd_client and netd. The netd checks
size of each FwmarkCommand. The earlier built netd does not expect that
ON_CONNECT command from libnetd_client includes connectInfo. It causes
connect() fail in the combination of new libnetd_client + old netd. With
this commit, the ON_CONNECT pass connectInfo only if OEM sets the
property ro.vendor.redirect_socket_calls. It's OEM responsibility to
ensure that the property is set only on the platform with netd that can
support connectInfo in ON_CONNECT command.

Minor changes:
1. Remove length protection on property retrieval. The
   __system_property_get() has built-in length protection. No need to
   append Null character.
2. refactor some code by macro.

Bug: 141611769
Bug: 150126287
Test: atest
Test: ./art/tools/run-libcore-tests.sh w/ current and earlier netd, pass
Test: ./art/tools/run-libjdwp-tests.sh w/ current and earlier netd, pass
Change-Id: I90ecba761effa0a5bc403876b9c3de8e77038232
Merged-In: I90ecba761effa0a5bc403876b9c3de8e77038232

server/FwmarkServer.cpp

diff --git a/server/FwmarkServer.cpp b/server/FwmarkServer.cpp
index 885db91..bc033fa 100644
--- a/server/FwmarkServer.cpp
+++ b/server/FwmarkServer.cpp
@@ -24,6 +24,7 @@
 
 #include <android-base/cmsg.h>
 #include <android-base/logging.h>
+#include <android-base/properties.h>
 #include <binder/IServiceManager.h>
 #include <netd_resolv/resolv.h>  // NETID_UNSET
 
@@ -66,7 +67,9 @@
     : SocketListener(SOCKET_NAME, true),
       mNetworkController(networkController),
       mEventReporter(eventReporter),
-      mTrafficCtrl(trafficCtrl) {}
+      mTrafficCtrl(trafficCtrl),
+      mRedirectSocketCalls(
+              android::base::GetBoolProperty("ro.vendor.redirect_socket_calls", false)) {}
 
 bool FwmarkServer::onDataAvailable(SocketClient* client) {
     int socketFd = -1;
@@ -85,13 +88,14 @@
     return false;
 }
 
-static bool hasDestinationAddress(FwmarkCommand::CmdId cmdId) {
-    if (cmdId == FwmarkCommand::ON_SENDTO || cmdId == FwmarkCommand::ON_CONNECT ||
-        cmdId == FwmarkCommand::ON_SENDMSG || cmdId == FwmarkCommand::ON_SENDMMSG ||
-        cmdId == FwmarkCommand::ON_CONNECT_COMPLETE) {
-        return true;
+static bool hasDestinationAddress(FwmarkCommand::CmdId cmdId, bool redirectSocketCalls) {
+    if (redirectSocketCalls) {
+        return (cmdId == FwmarkCommand::ON_SENDTO || cmdId == FwmarkCommand::ON_CONNECT ||
+                cmdId == FwmarkCommand::ON_SENDMSG || cmdId == FwmarkCommand::ON_SENDMMSG ||
+                cmdId == FwmarkCommand::ON_CONNECT_COMPLETE);
+    } else {
+        return (cmdId == FwmarkCommand::ON_CONNECT_COMPLETE);
     }
-    return false;
 }
 
 int FwmarkServer::processClient(SocketClient* client, int* socketFd) {
@@ -112,12 +116,12 @@
     memcpy(&command, buf, sizeof(command));
     memcpy(&connectInfo, buf + sizeof(command), sizeof(connectInfo));
 
-    size_t expected_len = sizeof(command);
-    if (hasDestinationAddress(command.cmdId)) {
-        expected_len += sizeof(connectInfo);
+    size_t expectedLen = sizeof(command);
+    if (hasDestinationAddress(command.cmdId, mRedirectSocketCalls)) {
+        expectedLen += sizeof(connectInfo);
     }
 
-    if (messageLength != static_cast<ssize_t>(expected_len)) {
+    if (messageLength != static_cast<ssize_t>(expectedLen)) {
         return -EBADMSG;
     }