diff options
author | Treehugger Robot <treehugger-gerrit@google.com> | 2021-06-17 10:42:58 +0000 |
---|---|---|
committer | Lorenzo Colitti <lorenzo@google.com> | 2021-06-17 13:25:22 +0000 |
commit | 9e41e9fba05f14596e682cb4a504b96bec7cda92 (patch) | |
tree | e7e3a328533096ec6f4cea50f788f4264f78c44b | |
parent | dbdd8f07ba0e3e079ccd3e7689231f85518f27bb (diff) | |
download | netd-9e41e9fba05f14596e682cb4a504b96bec7cda92.tar.gz |
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
-rw-r--r-- | server/NetlinkHandler.cpp | 6 | ||||
-rw-r--r-- | server/SockDiag.cpp | 25 | ||||
-rw-r--r-- | server/SockDiag.h | 4 | ||||
-rw-r--r-- | server/SockDiagTest.cpp | 2 | ||||
-rw-r--r-- | tests/binder_test.cpp | 67 |
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}, |