Specify the ifindex when destroying sockets on link-local.

Fix: 190477011
Test: new test coverage in netd_integration_test
Original-Change: https://android-review.googlesource.com/1738773
Merged-In: Id1e27b89580eda2fe7a235238c69366d94078e2c
Change-Id: Id1e27b89580eda2fe7a235238c69366d94078e2c

5 files changed, 91 insertions, 13 deletions

diff --git a/server/NetlinkHandler.cpp b/server/NetlinkHandler.cpp
index 7fb34379..525bb2d6 100644
--- a/server/NetlinkHandler.cpp
+++ b/server/NetlinkHandler.cpp
@@ -155,7 +155,11 @@ void NetlinkHandler::onEvent(NetlinkEvent *evt) {
                 if (shouldDestroy) {
                     SockDiag sd;
                     if (sd.open()) {
-                        int ret = sd.destroySockets(addrstr);
+                        // Pass the interface index iff. destroying sockets on a link-local address.
+                        // This cannot use an interface name as the interface might no longer exist.
+                        int destroyIfaceIndex =
+                                std::string_view(addrstr).starts_with("fe80:") ? ifaceIndex : 0;
+                        int ret = sd.destroySockets(addrstr, destroyIfaceIndex);
                         if (ret < 0) {
                             ALOGE("Error destroying sockets: %s", strerror(-ret));
                         }
diff --git a/server/SockDiag.cpp b/server/SockDiag.cpp
index e0b6b4b7..b3d9150a 100644
--- a/server/SockDiag.cpp
+++ b/server/SockDiag.cpp
@@ -32,6 +32,7 @@
 #include <cinttypes>
 
 #include <android-base/properties.h>
+#include <android-base/stringprintf.h>
 #include <android-base/strings.h>
 #include <log/log.h>
 #include <netdutils/InternetAddresses.h>
@@ -47,6 +48,7 @@
 
 namespace android {
 
+using android::base::StringPrintf;
 using netdutils::ScopedAddrinfo;
 using netdutils::Stopwatch;
 
@@ -304,7 +306,7 @@ int SockDiag::sockDestroy(uint8_t proto, const inet_diag_msg *msg) {
     return ret;
 }
 
-int SockDiag::destroySockets(uint8_t proto, int family, const char *addrstr) {
+int SockDiag::destroySockets(uint8_t proto, int family, const char* addrstr, int ifindex) {
     if (!hasSocks()) {
         return -EBADFD;
     }
@@ -313,28 +315,33 @@ int SockDiag::destroySockets(uint8_t proto, int family, const char *addrstr) {
         return ret;
     }
 
-    auto destroyAll = [] (uint8_t, const inet_diag_msg*) { return true; };
+    auto destroyAll = [ifindex](uint8_t, const inet_diag_msg* msg) {
+        return ifindex == 0 || ifindex == (int)msg->id.idiag_if;
+    };
 
     return readDiagMsg(proto, destroyAll);
 }
 
-int SockDiag::destroySockets(const char *addrstr) {
+int SockDiag::destroySockets(const char* addrstr, int ifindex) {
     Stopwatch s;
     mSocketsDestroyed = 0;
 
-    if (!strchr(addrstr, ':')) {
-        if (int ret = destroySockets(IPPROTO_TCP, AF_INET, addrstr)) {
-            ALOGE("Failed to destroy IPv4 sockets on %s: %s", addrstr, strerror(-ret));
+    std::string where = addrstr;
+    if (ifindex) where += StringPrintf(" ifindex %d", ifindex);
+
+    if (!strchr(addrstr, ':')) {  // inet_ntop never returns something like ::ffff:192.0.2.1
+        if (int ret = destroySockets(IPPROTO_TCP, AF_INET, addrstr, ifindex)) {
+            ALOGE("Failed to destroy IPv4 sockets on %s: %s", where.c_str(), strerror(-ret));
             return ret;
         }
     }
-    if (int ret = destroySockets(IPPROTO_TCP, AF_INET6, addrstr)) {
-        ALOGE("Failed to destroy IPv6 sockets on %s: %s", addrstr, strerror(-ret));
+    if (int ret = destroySockets(IPPROTO_TCP, AF_INET6, addrstr, ifindex)) {
+        ALOGE("Failed to destroy IPv6 sockets on %s: %s", where.c_str(), strerror(-ret));
         return ret;
     }
 
     if (mSocketsDestroyed > 0) {
-        ALOGI("Destroyed %d sockets on %s in %" PRId64 "us", mSocketsDestroyed, addrstr,
+        ALOGI("Destroyed %d sockets on %s in %" PRId64 "us", mSocketsDestroyed, where.c_str(),
               s.timeTakenUs());
     }
 
diff --git a/server/SockDiag.h b/server/SockDiag.h
index 745c09e3..240e4e5d 100644
--- a/server/SockDiag.h
+++ b/server/SockDiag.h
@@ -70,7 +70,7 @@ class SockDiag {
 
     int sockDestroy(uint8_t proto, const inet_diag_msg *);
     // Destroys all sockets on the given IPv4 or IPv6 address.
-    int destroySockets(const char *addrstr);
+    int destroySockets(const char* addrstr, int ifindex);
     // Destroys all sockets for the given protocol and UID.
     int destroySockets(uint8_t proto, uid_t uid, bool excludeLoopback);
     // Destroys all "live" (CONNECTED, SYN_SENT, SYN_RECV) TCP sockets for the given UID ranges.
@@ -91,7 +91,7 @@ class SockDiag {
     int mSocketsDestroyed;
     int sendDumpRequest(uint8_t proto, uint8_t family, uint8_t extensions, uint32_t states,
                         iovec *iov, int iovcnt);
-    int destroySockets(uint8_t proto, int family, const char *addrstr);
+    int destroySockets(uint8_t proto, int family, const char* addrstr, int ifindex);
     int destroyLiveSockets(const DestroyFilter& destroy, const char *what, iovec *iov, int iovcnt);
     bool hasSocks() { return mSock != -1 && mWriteSock != -1; }
     void closeSocks() { close(mSock); close(mWriteSock); mSock = mWriteSock = -1; }
diff --git a/server/SockDiagTest.cpp b/server/SockDiagTest.cpp
index b79471a5..49601aa4 100644
--- a/server/SockDiagTest.cpp
+++ b/server/SockDiagTest.cpp
@@ -370,7 +370,7 @@ protected:
         int ret;
         switch (mode) {
             case ADDRESS:
-                ret = mSd.destroySockets("::1");
+                ret = mSd.destroySockets("::1", 0 /* ifindex */);
                 EXPECT_LE(0, ret) << ": Failed to destroy sockets on ::1: " << strerror(-ret);
                 break;
             case UID:
diff --git a/tests/binder_test.cpp b/tests/binder_test.cpp
index 22cba742..69d1f9b5 100644
--- a/tests/binder_test.cpp
+++ b/tests/binder_test.cpp
@@ -750,6 +750,72 @@ TEST_F(NetdBinderTest, SocketDestroy) {
     checkSocketpairClosed(clientSocket, acceptedSocket);
 }
 
+TEST_F(NetdBinderTest, SocketDestroyLinkLocal) {
+    // Add the same link-local address to two interfaces.
+    const char* kLinkLocalAddress = "fe80::ace:d00d";
+
+    const struct addrinfo hints = {
+            .ai_family = AF_INET6,
+            .ai_socktype = SOCK_STREAM,
+            .ai_flags = AI_NUMERICHOST,
+    };
+
+    binder::Status status = mNetd->interfaceAddAddress(sTun.name(), kLinkLocalAddress, 64);
+    EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+    status = mNetd->interfaceAddAddress(sTun2.name(), kLinkLocalAddress, 64);
+    EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+
+    // Bind a listening socket to the address on each of two interfaces.
+    // The sockets must be open at the same time, because this test checks that SOCK_DESTROY only
+    // destroys the sockets on the interface where the address is deleted.
+    struct addrinfo* addrinfoList = nullptr;
+    int ret = getaddrinfo(kLinkLocalAddress, nullptr, &hints, &addrinfoList);
+    ScopedAddrinfo addrinfoCleanup(addrinfoList);
+    ASSERT_EQ(0, ret);
+
+    socklen_t len = addrinfoList[0].ai_addrlen;
+    sockaddr_in6 sin6_1 = *reinterpret_cast<sockaddr_in6*>(addrinfoList[0].ai_addr);
+    sockaddr_in6 sin6_2 = sin6_1;
+    sin6_1.sin6_scope_id = if_nametoindex(sTun.name().c_str());
+    sin6_2.sin6_scope_id = if_nametoindex(sTun2.name().c_str());
+
+    int s1 = socket(AF_INET6, SOCK_STREAM, 0);
+    ASSERT_EQ(0, bind(s1, reinterpret_cast<sockaddr*>(&sin6_1), len));
+    ASSERT_EQ(0, getsockname(s1, reinterpret_cast<sockaddr*>(&sin6_1), &len));
+
+    int s2 = socket(AF_INET6, SOCK_STREAM, 0);
+    ASSERT_EQ(0, bind(s2, reinterpret_cast<sockaddr*>(&sin6_2), len));
+    ASSERT_EQ(0, getsockname(s2, reinterpret_cast<sockaddr*>(&sin6_2), &len));
+
+    ASSERT_EQ(0, listen(s1, 10));
+    ASSERT_EQ(0, listen(s2, 10));
+
+    // Connect one client socket to each and accept the connections.
+    int c1 = socket(AF_INET6, SOCK_STREAM, 0);
+    int c2 = socket(AF_INET6, SOCK_STREAM, 0);
+    ASSERT_EQ(0, connect(c1, reinterpret_cast<sockaddr*>(&sin6_1), len));
+    ASSERT_EQ(0, connect(c2, reinterpret_cast<sockaddr*>(&sin6_2), len));
+    int a1 = accept(s1, nullptr, 0);
+    ASSERT_NE(-1, a1);
+    int a2 = accept(s2, nullptr, 0);
+    ASSERT_NE(-1, a2);
+
+    // Delete the address on sTun2.
+    status = mNetd->interfaceDelAddress(sTun2.name(), kLinkLocalAddress, 64);
+    EXPECT_TRUE(status.isOk()) << status.exceptionMessage();
+
+    // The sockets on sTun2 are closed, but the ones on sTun1 remain open.
+    char buf[1024];
+    EXPECT_EQ(-1, read(c2, buf, sizeof(buf)));
+    EXPECT_EQ(ECONNABORTED, errno);
+    // The blocking read above ensures that SOCK_DESTROY has completed.
+
+    EXPECT_EQ(3, write(a1, "foo", 3));
+    EXPECT_EQ(3, read(c1, buf, sizeof(buf)));
+    EXPECT_EQ(-1, write(a2, "foo", 3));
+    EXPECT_TRUE(errno == ECONNABORTED || errno == ECONNRESET);
+}
+
 namespace {
 
 int netmaskToPrefixLength(const uint8_t *buf, size_t buflen) {
@@ -868,6 +934,7 @@ TEST_F(NetdBinderTest, InterfaceAddRemoveAddress) {
             {"2001:db8::1", 64, 0, 0},
             {"2001:db8::2", 65, 0, 0},
             {"2001:db8::3", 128, 0, 0},
+            {"fe80::1234", 64, 0, 0},
             {"2001:db8::4", 129, EINVAL, EINVAL},
             {"foo:bar::bad", 64, EINVAL, EINVAL},
             {"2001:db8::1/64", 64, EINVAL, EINVAL},