adb: fix fd double close, Subprocess lifetime issue. (9150b128) · Commits · e / os / android_system_core

adb/adb_utils.h

+9 −0

Original line number	Diff line number	Diff line
		@@ -20,6 +20,7 @@
		#include <string>

		#include <android-base/macros.h>
		#include <android-base/unique_fd.h>

		void close_stdin();

		@@ -50,6 +51,14 @@ bool set_file_block_mode(int fd, bool block);
		extern int adb_close(int fd);

		// Helper to automatically close an FD when it goes out of scope.
		struct AdbCloser {
		static void Close(int fd) {
		adb_close(fd);
		}
		};

		using unique_fd = android::base::unique_fd_impl<AdbCloser>;

		class ScopedFd {
		public:
		ScopedFd() {

adb/shell_service.cpp

+26 −9

Original line number	Diff line number	Diff line
		@@ -160,9 +160,14 @@ class Subprocess {
		pid_t pid() const { return pid_; }

		// Sets up FDs, forks a subprocess, starts the subprocess manager thread,
		// and exec's the child. Returns false on failure.
		// and exec's the child. Returns false and sets error on failure.
		bool ForkAndExec(std::string* _Nonnull error);

		// Start the subprocess manager thread. Consumes the subprocess, regardless of success.
		// Returns false and sets error on failure.
		static bool StartThread(std::unique_ptr<Subprocess> subprocess,
		std::string* _Nonnull error);

		private:
		// Opens the file at \|pts_name\|.
		int OpenPtyChildFd(const char* pts_name, ScopedFd* error_sfd);
		@@ -391,14 +396,19 @@ bool Subprocess::ForkAndExec(std::string* error) {
		}
		}

		if (!adb_thread_create(ThreadHandler, this)) {
		D("subprocess parent: completed");
		return true;
		}

		bool Subprocess::StartThread(std::unique_ptr<Subprocess> subprocess, std::string* error) {
		Subprocess* raw = subprocess.release();
		if (!adb_thread_create(ThreadHandler, raw)) {
		*error =
		android::base::StringPrintf("failed to create subprocess thread: %s", strerror(errno));
		kill(pid_, SIGKILL);
		kill(raw->pid_, SIGKILL);
		return false;
		}

		D("subprocess parent: completed");
		return true;
		}

		@@ -442,6 +452,7 @@ void Subprocess::ThreadHandler(void* userdata) {
		adb_thread_setname(android::base::StringPrintf(
		"shell srvc %d", subprocess->local_socket_fd()));

		D("passing data streams for PID %d", subprocess->pid());
		subprocess->PassDataStreams();

		D("deleting Subprocess for PID %d", subprocess->pid());
		@@ -738,7 +749,7 @@ int StartSubprocess(const char* name, const char* terminal_type,
		protocol == SubprocessProtocol::kNone ? "none" : "shell",
		terminal_type, name);

		Subprocess* subprocess = new Subprocess(name, terminal_type, type, protocol);
		auto subprocess = std::make_unique<Subprocess>(name, terminal_type, type, protocol);
		if (!subprocess) {
		LOG(ERROR) << "failed to allocate new subprocess";
		return ReportError(protocol, "failed to allocate new subprocess");
		@@ -747,11 +758,17 @@ int StartSubprocess(const char* name, const char* terminal_type,
		std::string error;
		if (!subprocess->ForkAndExec(&error)) {
		LOG(ERROR) << "failed to start subprocess: " << error;
		delete subprocess;
		return ReportError(protocol, error);
		}

		D("subprocess creation successful: local_socket_fd=%d, pid=%d",
		subprocess->local_socket_fd(), subprocess->pid());
		return subprocess->local_socket_fd();
		unique_fd local_socket(dup(subprocess->local_socket_fd()));
		D("subprocess creation successful: local_socket_fd=%d, pid=%d", local_socket.get(),
		subprocess->pid());

		if (!Subprocess::StartThread(std::move(subprocess), &error)) {
		LOG(ERROR) << "failed to start subprocess management thread: " << error;
		return ReportError(protocol, error);
		}

		return local_socket.release();
		}

adb/sysdeps.h

+2 −1

Original line number	Diff line number	Diff line
		@@ -29,8 +29,9 @@
		#include <string>
		#include <vector>

		// Include this before open/unlink are defined as macros below.
		// Include this before open/close/unlink are defined as macros below.
		#include <android-base/errors.h>
		#include <android-base/unique_fd.h>
		#include <android-base/utf8.h>

		/*

base/include/android-base/errors.h

+3 −3

Original line number	Diff line number	Diff line
		@@ -27,8 +27,8 @@
		// special handling to get the error string. Refer to Microsoft documentation
		// to determine which error code to check for each function.

		#ifndef BASE_ERRORS_H
		#define BASE_ERRORS_H
		#ifndef ANDROID_BASE_ERRORS_H
		#define ANDROID_BASE_ERRORS_H

		#include <string>

		@@ -43,4 +43,4 @@ std::string SystemErrorCodeToString(int error_code);
		} // namespace base
		} // namespace android

		#endif // BASE_ERRORS_H
		#endif // ANDROID_BASE_ERRORS_H

base/include/android-base/file.h

+3 −3

Original line number	Diff line number	Diff line
		@@ -14,8 +14,8 @@
		* limitations under the License.
		*/

		#ifndef BASE_FILE_H
		#define BASE_FILE_H
		#ifndef ANDROID_BASE_FILE_H
		#define ANDROID_BASE_FILE_H

		#include <sys/stat.h>
		#include <string>
		@@ -46,4 +46,4 @@ bool RemoveFileIfExists(const std::string& path, std::string* err = nullptr);
		} // namespace base
		} // namespace android

		#endif // BASE_FILE_H
		#endif // ANDROID_BASE_FILE_H