Fixing logic for using android_setsocknetwork() with bind().

webrtc/base/physicalsocketserver.cc

Index: webrtc/base/physicalsocketserver.cc
diff --git a/webrtc/base/physicalsocketserver.cc b/webrtc/base/physicalsocketserver.cc
index e2ec589eb12d0229febab295fe420472957f3a9b..a963ca204a536d8714f78f0a3edbb05dac1f73ef 100644
--- a/webrtc/base/physicalsocketserver.cc
+++ b/webrtc/base/physicalsocketserver.cc
@@ -189,8 +189,34 @@ SocketAddress PhysicalSocket::GetRemoteAddress() const {
 }
 
 int PhysicalSocket::Bind(const SocketAddress& bind_addr) {
+  SocketAddress copied_bind_addr = bind_addr;
+  // If a network binder is available, use it to bind a socket to an interface
+  // instead of bind(), since this is more reliable on an OS with a weak host
+  // model.
+  if (ss_->network_binder()) {
+    NetworkBindingResult result =
+        ss_->network_binder()->BindSocketToNetwork(s_, bind_addr.ipaddr());
+    if (result == NetworkBindingResult::SUCCESS) {
+      // Since the network binder handled binding the socket to the desired
+      // network interface, we don't need to (and shouldn't) include an IP in
+      // the bind() call; bind() just needs to assign a port.
+      copied_bind_addr.SetIP(GetAnyIP(copied_bind_addr.ipaddr().family()));
+    } else if (result == NetworkBindingResult::NOT_IMPLEMENTED) {
+      LOG(LS_INFO) << "Can't bind socket to network because "
+                      "network binding is not implemented for this OS.";
+    } else {
+      LOG(LS_WARNING) << "Binding socket to network address "
+                      << bind_addr.ipaddr().ToString()
+                      << " failed; result: " << static_cast<int>(result);
+      // If a network binding was attempted and failed, we should stop here and
+      // not try to use the socket. Otherwise, we may end up sending packets
+      // with an invalid source address.
+      // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=7026
+      return -1;

[pthatcher1, 2017/02/10 18:12:19]

Should we return different errors based on the result?  In fact, should we
return a NetworkBindingResult from this method?

[Taylor Brandstetter, 2017/02/10 18:57:52]

Bind either returns -1 or 0, and I wasn't planning to change that in this CL.
There would be no point in returning a NetworkBindError now since nothing would
use it.

+    }
+  }
   sockaddr_storage addr_storage;
-  size_t len = bind_addr.ToSockAddrStorage(&addr_storage);
+  size_t len = copied_bind_addr.ToSockAddrStorage(&addr_storage);
   sockaddr* addr = reinterpret_cast<sockaddr*>(&addr_storage);
   int err = ::bind(s_, addr, static_cast<int>(len));
   UpdateLastError();
@@ -200,14 +226,6 @@ int PhysicalSocket::Bind(const SocketAddress& bind_addr) {
     dbg_addr_.append(GetLocalAddress().ToString());
   }
 #endif
-  if (ss_->network_binder()) {
-    int result =
-        ss_->network_binder()->BindSocketToNetwork(s_, bind_addr.ipaddr());
-    if (result < 0) {
-      LOG(LS_INFO) << "Binding socket to network address "
-                   << bind_addr.ipaddr().ToString() << " result " << result;
-    }
-  }
   return err;
 }