From 297d9bf8ea41e6eb8b50000163a13d30f6478c7e Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 6 Aug 2018 18:38:47 -0700 Subject: [PATCH] adb: actually enable fdsan. adb was using a custom unique_fd closer that didn't have an implementation for fdsan, which meant that none of our FDs were actually tracked. Guard this behind ifdefs so that we only use this on Windows, and delete our implementation of Pipe in favor of the one in libbase while we're at it. libbase's implementation always sets O_CLOEXEC, so fix up the instance of Pipe that doesn't expect that. Test: mma Test: adb start-server Test: debuggerd `pidof adbd` Change-Id: Ic29d641a2f93fb42384b00c51775048c8bcbe152 --- adb/adb.cpp | 4 ++++ adb/adb_unique_fd.cpp | 43 +------------------------------------------ adb/adb_unique_fd.h | 6 +++--- 3 files changed, 8 insertions(+), 45 deletions(-) diff --git a/adb/adb.cpp b/adb/adb.cpp index 38c6f62c9..a8fe73651 100644 --- a/adb/adb.cpp +++ b/adb/adb.cpp @@ -891,6 +891,10 @@ int launch_server(const std::string& socket_spec) { // child side of the fork pipe_read.reset(); + // android::base::Pipe unconditionally opens the pipe with O_CLOEXEC. + // Undo this manually. + fcntl(pipe_write.get(), F_SETFD, 0); + char reply_fd[30]; snprintf(reply_fd, sizeof(reply_fd), "%d", pipe_write.get()); // child process diff --git a/adb/adb_unique_fd.cpp b/adb/adb_unique_fd.cpp index 58e768e13..dec73bc2e 100644 --- a/adb/adb_unique_fd.cpp +++ b/adb/adb_unique_fd.cpp @@ -21,49 +21,8 @@ #include "sysdeps.h" +#if defined(_WIN32) void AdbCloser::Close(int fd) { adb_close(fd); } - -#if !defined(_WIN32) -bool Pipe(unique_fd* read, unique_fd* write, int flags) { - int pipefd[2]; -#if !defined(__APPLE__) - if (pipe2(pipefd, flags) != 0) { - return false; - } -#else - // Darwin doesn't have pipe2. Implement it ourselves. - if (flags != 0 && (flags & ~(O_CLOEXEC | O_NONBLOCK)) != 0) { - errno = EINVAL; - return false; - } - - if (pipe(pipefd) != 0) { - return false; - } - - if (flags & O_CLOEXEC) { - if (fcntl(pipefd[0], F_SETFD, FD_CLOEXEC) != 0 || - fcntl(pipefd[1], F_SETFD, FD_CLOEXEC) != 0) { - adb_close(pipefd[0]); - adb_close(pipefd[1]); - return false; - } - } - - if (flags & O_NONBLOCK) { - if (fcntl(pipefd[0], F_SETFL, O_NONBLOCK) != 0 || - fcntl(pipefd[1], F_SETFL, O_NONBLOCK) != 0) { - adb_close(pipefd[0]); - adb_close(pipefd[1]); - return false; - } - } -#endif - - read->reset(pipefd[0]); - write->reset(pipefd[1]); - return true; -} #endif diff --git a/adb/adb_unique_fd.h b/adb/adb_unique_fd.h index bad501abb..d47213d75 100644 --- a/adb/adb_unique_fd.h +++ b/adb/adb_unique_fd.h @@ -21,15 +21,15 @@ #include +#if defined(_WIN32) // Helper to automatically close an FD when it goes out of scope. struct AdbCloser { static void Close(int fd); }; using unique_fd = android::base::unique_fd_impl; - -#if !defined(_WIN32) -bool Pipe(unique_fd* read, unique_fd* write, int flags = 0); +#else +using unique_fd = android::base::unique_fd; #endif template