[CLATJ#29] Remove the throwaway sockets for dup2 fds
Don't need these temporary socket fds anymore because the
posix_spawn_file_actions_adddup2() will clean up the flag
O_CLOEXEC for dup2 fd == new_fd before forking.
Bug: 218407445
Test: connect to ipv6 only network and ping 8.8.8.8
Change-Id: I70b8a03a6a13393a30a8a63599b8d1468347c560
diff --git a/service/jni/com_android_server_connectivity_ClatCoordinator.cpp b/service/jni/com_android_server_connectivity_ClatCoordinator.cpp
index ee512ec..71ddd11 100644
--- a/service/jni/com_android_server_connectivity_ClatCoordinator.cpp
+++ b/service/jni/com_android_server_connectivity_ClatCoordinator.cpp
@@ -318,30 +318,13 @@
return -1;
}
- // 1. create a throwaway socket to reserve a file descriptor number
- int passedTunFd = socket(AF_INET6, SOCK_DGRAM | SOCK_CLOEXEC, 0);
- if (passedTunFd == -1) {
- throwIOException(env, "socket(ipv6/udp) for tun fd failed", errno);
- return -1;
- }
- int passedSockRead = socket(AF_INET6, SOCK_DGRAM | SOCK_CLOEXEC, 0);
- if (passedSockRead == -1) {
- throwIOException(env, "socket(ipv6/udp) for read socket failed", errno);
- return -1;
- }
- int passedSockWrite = socket(AF_INET6, SOCK_DGRAM | SOCK_CLOEXEC, 0);
- if (passedSockWrite == -1) {
- throwIOException(env, "socket(ipv6/udp) for write socket failed", errno);
- return -1;
- }
-
- // these are the FD we'll pass to clatd on the cli, so need it as a string
- char passedTunFdStr[INT32_STRLEN];
- char passedSockReadStr[INT32_STRLEN];
- char passedSockWriteStr[INT32_STRLEN];
- snprintf(passedTunFdStr, sizeof(passedTunFdStr), "%d", passedTunFd);
- snprintf(passedSockReadStr, sizeof(passedSockReadStr), "%d", passedSockRead);
- snprintf(passedSockWriteStr, sizeof(passedSockWriteStr), "%d", passedSockWrite);
+ // 1. these are the FD we'll pass to clatd on the cli, so need it as a string
+ char tunFdStr[INT32_STRLEN];
+ char sockReadStr[INT32_STRLEN];
+ char sockWriteStr[INT32_STRLEN];
+ snprintf(tunFdStr, sizeof(tunFdStr), "%d", tunFd);
+ snprintf(sockReadStr, sizeof(sockReadStr), "%d", readSock);
+ snprintf(sockWriteStr, sizeof(sockWriteStr), "%d", writeSock);
// 2. we're going to use this as argv[0] to clatd to make ps output more useful
std::string progname("clatd-");
@@ -353,9 +336,9 @@
"-p", pfx96Str.c_str(),
"-4", v4Str.c_str(),
"-6", v6Str.c_str(),
- "-t", passedTunFdStr,
- "-r", passedSockReadStr,
- "-w", passedSockWriteStr,
+ "-t", tunFdStr,
+ "-r", sockReadStr,
+ "-w", sockWriteStr,
nullptr};
// clang-format on
@@ -375,7 +358,9 @@
// 4. register dup2() action: this is what 'clears' the CLOEXEC flag
// on the tun fd that we want the child clatd process to inherit
- // (this will happen after the vfork, and before the execve)
+ // (this will happen after the vfork, and before the execve).
+ // Note that even though dup2(2) is a no-op if fd == new_fd but O_CLOEXEC flag will be removed.
+ // See implementation of bionic's posix_spawn_file_actions_adddup2().
posix_spawn_file_actions_t fa;
if (int ret = posix_spawn_file_actions_init(&fa)) {
posix_spawnattr_destroy(&attr);
@@ -383,19 +368,19 @@
return -1;
}
- if (int ret = posix_spawn_file_actions_adddup2(&fa, tunFd, passedTunFd)) {
+ if (int ret = posix_spawn_file_actions_adddup2(&fa, tunFd, tunFd)) {
posix_spawnattr_destroy(&attr);
posix_spawn_file_actions_destroy(&fa);
throwIOException(env, "posix_spawn_file_actions_adddup2 for tun fd failed", ret);
return -1;
}
- if (int ret = posix_spawn_file_actions_adddup2(&fa, readSock, passedSockRead)) {
+ if (int ret = posix_spawn_file_actions_adddup2(&fa, readSock, readSock)) {
posix_spawnattr_destroy(&attr);
posix_spawn_file_actions_destroy(&fa);
throwIOException(env, "posix_spawn_file_actions_adddup2 for read socket failed", ret);
return -1;
}
- if (int ret = posix_spawn_file_actions_adddup2(&fa, writeSock, passedSockWrite)) {
+ if (int ret = posix_spawn_file_actions_adddup2(&fa, writeSock, writeSock)) {
posix_spawnattr_destroy(&attr);
posix_spawn_file_actions_destroy(&fa);
throwIOException(env, "posix_spawn_file_actions_adddup2 for write socket failed", ret);
@@ -414,7 +399,7 @@
posix_spawnattr_destroy(&attr);
posix_spawn_file_actions_destroy(&fa);
- // 5. Start BPF if any
+ // 6. Start BPF if any
if (!net::clat::initMaps()) {
net::clat::ClatdTracker tracker = {};
if (!initTracker(ifaceStr.c_str(), pfx96Str.c_str(), v4Str.c_str(), v6Str.c_str(),