From a2f6d748a24879d0bdd824e2020d0938d103c8f9 Mon Sep 17 00:00:00 2001 From: rickyz Date: Wed, 21 Jan 2015 13:57:34 -0800 Subject: [PATCH] Move ForkWithFlags from process* to launch*. As suggested at https://codereview.chromium.org/840893003/. BUG= Review URL: https://codereview.chromium.org/839013005 Cr-Commit-Position: refs/heads/master@{#312465} --- base/process/launch.h | 19 ++++++ base/process/launch_posix.cc | 94 +++++++++++++++++++++++++++ base/process/process.h | 19 ------ base/process/process_linux.cc | 92 --------------------------- base/process/process_unittest.cc | 116 ---------------------------------- base/process/process_util_unittest.cc | 110 ++++++++++++++++++++++++++++++++ content/zygote/zygote_linux.cc | 4 +- sandbox/linux/services/credentials.cc | 2 +- 8 files changed, 227 insertions(+), 229 deletions(-) diff --git a/base/process/launch.h b/base/process/launch.h index eca4f8f7a44e..de1bc0af821f 100644 --- a/base/process/launch.h +++ b/base/process/launch.h @@ -291,6 +291,25 @@ void ReplaceBootstrapPort(const std::string& replacement_bootstrap_name); // binary. This should not be called in production/released code. BASE_EXPORT LaunchOptions LaunchOptionsForTest(); +#if defined(OS_LINUX) +// A wrapper for clone with fork-like behavior, meaning that it returns the +// child's pid in the parent and 0 in the child. |flags|, |ptid|, and |ctid| are +// as in the clone system call (the CLONE_VM flag is not supported). +// +// This function uses the libc clone wrapper (which updates libc's pid cache) +// internally, so callers may expect things like getpid() to work correctly +// after in both the child and parent. An exception is when this code is run +// under Valgrind. Valgrind does not support the libc clone wrapper, so the libc +// pid cache may be incorrect after this function is called under Valgrind. +// +// As with fork(), callers should be extremely careful when calling this while +// multiple threads are running, since at the time the fork happened, the +// threads could have been in any state (potentially holding locks, etc.). +// Callers should most likely call execve() in the child soon after calling +// this. +BASE_EXPORT pid_t ForkWithFlags(unsigned long flags, pid_t* ptid, pid_t* ctid); +#endif + } // namespace base #endif // BASE_PROCESS_LAUNCH_H_ diff --git a/base/process/launch_posix.cc b/base/process/launch_posix.cc index 6c9ed3ff3db8..ce93d5056fd8 100644 --- a/base/process/launch_posix.cc +++ b/base/process/launch_posix.cc @@ -7,9 +7,12 @@ #include #include #include +#include +#include #include #include #include +#include #include #include #include @@ -35,8 +38,10 @@ #include "base/strings/stringprintf.h" #include "base/synchronization/waitable_event.h" #include "base/third_party/dynamic_annotations/dynamic_annotations.h" +#include "base/third_party/valgrind/valgrind.h" #include "base/threading/platform_thread.h" #include "base/threading/thread_restrictions.h" +#include "build/build_config.h" #if defined(OS_LINUX) #include @@ -184,6 +189,54 @@ void ResetChildSignalHandlersToDefaults(void) { #endif // !defined(OS_LINUX) || // (!defined(__i386__) && !defined(__x86_64__) && !defined(__arm__)) +#if defined(OS_LINUX) +bool IsRunningOnValgrind() { + return RUNNING_ON_VALGRIND; +} + +// This function runs on the stack specified on the clone call. It uses longjmp +// to switch back to the original stack so the child can return from sys_clone. +int CloneHelper(void* arg) { + jmp_buf* env_ptr = reinterpret_cast(arg); + longjmp(*env_ptr, 1); + + // Should not be reached. + RAW_CHECK(false); + return 1; +} + +// This function is noinline to ensure that stack_buf is below the stack pointer +// that is saved when setjmp is called below. This is needed because when +// compiled with FORTIFY_SOURCE, glibc's longjmp checks that the stack is moved +// upwards. See crbug.com/442912 for more details. +#if defined(ADDRESS_SANITIZER) +// Disable AddressSanitizer instrumentation for this function to make sure +// |stack_buf| is allocated on thread stack instead of ASan's fake stack. +// Under ASan longjmp() will attempt to clean up the area between the old and +// new stack pointers and print a warning that may confuse the user. +__attribute__((no_sanitize_address)) +#endif +NOINLINE pid_t CloneAndLongjmpInChild(unsigned long flags, + pid_t* ptid, + pid_t* ctid, + jmp_buf* env) { + // We use the libc clone wrapper instead of making the syscall + // directly because making the syscall may fail to update the libc's + // internal pid cache. The libc interface unfortunately requires + // specifying a new stack, so we use setjmp/longjmp to emulate + // fork-like behavior. + char stack_buf[PTHREAD_STACK_MIN]; +#if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARM_FAMILY) || \ + defined(ARCH_CPU_MIPS64_FAMILY) || defined(ARCH_CPU_MIPS_FAMILY) + // The stack grows downward. + void* stack = stack_buf + sizeof(stack_buf); +#else +#error "Unsupported architecture" +#endif + return clone(&CloneHelper, stack, flags, env, ptid, nullptr, ctid); +} +#endif // defined(OS_LINUX) + } // anonymous namespace // Functor for |ScopedDIR| (below). @@ -671,4 +724,45 @@ bool GetAppOutputWithExitCode(const CommandLine& cl, return result == EXECUTE_SUCCESS; } +#if defined(OS_LINUX) +pid_t ForkWithFlags(unsigned long flags, pid_t* ptid, pid_t* ctid) { + const bool clone_tls_used = flags & CLONE_SETTLS; + const bool invalid_ctid = + (flags & (CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID)) && !ctid; + const bool invalid_ptid = (flags & CLONE_PARENT_SETTID) && !ptid; + + // We do not support CLONE_VM. + const bool clone_vm_used = flags & CLONE_VM; + + if (clone_tls_used || invalid_ctid || invalid_ptid || clone_vm_used) { + RAW_LOG(FATAL, "Invalid usage of ForkWithFlags"); + } + + // Valgrind's clone implementation does not support specifiying a child_stack + // without CLONE_VM, so we cannot use libc's clone wrapper when running under + // Valgrind. As a result, the libc pid cache may be incorrect under Valgrind. + // See crbug.com/442817 for more details. + if (IsRunningOnValgrind()) { + // See kernel/fork.c in Linux. There is different ordering of sys_clone + // parameters depending on CONFIG_CLONE_BACKWARDS* configuration options. +#if defined(ARCH_CPU_X86_64) + return syscall(__NR_clone, flags, nullptr, ptid, ctid, nullptr); +#elif defined(ARCH_CPU_X86) || defined(ARCH_CPU_ARM_FAMILY) || \ + defined(ARCH_CPU_MIPS_FAMILY) || defined(ARCH_CPU_MIPS64_FAMILY) + // CONFIG_CLONE_BACKWARDS defined. + return syscall(__NR_clone, flags, nullptr, ptid, nullptr, ctid); +#else +#error "Unsupported architecture" +#endif + } + + jmp_buf env; + if (setjmp(env) == 0) { + return CloneAndLongjmpInChild(flags, ptid, ctid, &env); + } + + return 0; +} +#endif // defined(OS_LINUX) + } // namespace base diff --git a/base/process/process.h b/base/process/process.h index b0fd8d909ec8..d556d8e0981a 100644 --- a/base/process/process.h +++ b/base/process/process.h @@ -117,25 +117,6 @@ class BASE_EXPORT Process { #endif }; -#if defined(OS_LINUX) -// A wrapper for clone with fork-like behavior, meaning that it returns the -// child's pid in the parent and 0 in the child. |flags|, |ptid|, and |ctid| are -// as in the clone system call (the CLONE_VM flag is not supported). -// -// This function uses the libc clone wrapper (which updates libc's pid cache) -// internally, so callers may expect things like getpid() to work correctly -// after in both the child and parent. An exception is when this code is run -// under Valgrind. Valgrind does not support the libc clone wrapper, so the libc -// pid cache may be incorrect after this function is called under Valgrind. -// -// As with fork(), callers should be extremely careful when calling this while -// multiple threads are running, since at the time the fork happened, the -// threads could have been in any state (potentially holding locks, etc.). -// Callers should most likely call execve() in the child soon after calling -// this. -BASE_EXPORT pid_t ForkWithFlags(unsigned long flags, pid_t* ptid, pid_t* ctid); -#endif - } // namespace base #endif // BASE_PROCESS_PROCESS_PROCESS_H_ diff --git a/base/process/process_linux.cc b/base/process/process_linux.cc index cfa3ed5fd07f..59ee288f8801 100644 --- a/base/process/process_linux.cc +++ b/base/process/process_linux.cc @@ -5,21 +5,14 @@ #include "base/process/process.h" #include -#include -#include -#include #include -#include -#include "base/compiler_specific.h" #include "base/files/file_util.h" #include "base/lazy_instance.h" #include "base/logging.h" #include "base/strings/string_split.h" #include "base/strings/stringprintf.h" #include "base/synchronization/lock.h" -#include "base/third_party/valgrind/valgrind.h" -#include "build/build_config.h" namespace base { @@ -85,52 +78,6 @@ struct CheckForNicePermission { bool can_reraise_priority; }; -bool IsRunningOnValgrind() { - return RUNNING_ON_VALGRIND; -} - -// This function runs on the stack specified on the clone call. It uses longjmp -// to switch back to the original stack so the child can return from sys_clone. -int CloneHelper(void* arg) { - jmp_buf* env_ptr = reinterpret_cast(arg); - longjmp(*env_ptr, 1); - - // Should not be reached. - RAW_CHECK(false); - return 1; -} - -// This function is noinline to ensure that stack_buf is below the stack pointer -// that is saved when setjmp is called below. This is needed because when -// compiled with FORTIFY_SOURCE, glibc's longjmp checks that the stack is moved -// upwards. See crbug.com/442912 for more details. -#if defined(ADDRESS_SANITIZER) -// Disable AddressSanitizer instrumentation for this function to make sure -// |stack_buf| is allocated on thread stack instead of ASan's fake stack. -// Under ASan longjmp() will attempt to clean up the area between the old and -// new stack pointers and print a warning that may confuse the user. -__attribute__((no_sanitize_address)) -#endif -NOINLINE pid_t CloneAndLongjmpInChild(unsigned long flags, - pid_t* ptid, - pid_t* ctid, - jmp_buf* env) { - // We use the libc clone wrapper instead of making the syscall - // directly because making the syscall may fail to update the libc's - // internal pid cache. The libc interface unfortunately requires - // specifying a new stack, so we use setjmp/longjmp to emulate - // fork-like behavior. - char stack_buf[PTHREAD_STACK_MIN]; -#if defined(ARCH_CPU_X86_FAMILY) || defined(ARCH_CPU_ARM_FAMILY) || \ - defined(ARCH_CPU_MIPS64_FAMILY) || defined(ARCH_CPU_MIPS_FAMILY) - // The stack grows downward. - void* stack = stack_buf + sizeof(stack_buf); -#else -#error "Unsupported architecture" -#endif - return clone(&CloneHelper, stack, flags, env, ptid, nullptr, ctid); -} - } // namespace // static @@ -189,43 +136,4 @@ bool Process::SetProcessBackgrounded(bool background) { return result == 0; } -pid_t ForkWithFlags(unsigned long flags, pid_t* ptid, pid_t* ctid) { - const bool clone_tls_used = flags & CLONE_SETTLS; - const bool invalid_ctid = - (flags & (CLONE_CHILD_SETTID | CLONE_CHILD_CLEARTID)) && !ctid; - const bool invalid_ptid = (flags & CLONE_PARENT_SETTID) && !ptid; - - // We do not support CLONE_VM. - const bool clone_vm_used = flags & CLONE_VM; - - if (clone_tls_used || invalid_ctid || invalid_ptid || clone_vm_used) { - RAW_LOG(FATAL, "Invalid usage of ForkWithFlags"); - } - - // Valgrind's clone implementation does not support specifiying a child_stack - // without CLONE_VM, so we cannot use libc's clone wrapper when running under - // Valgrind. As a result, the libc pid cache may be incorrect under Valgrind. - // See crbug.com/442817 for more details. - if (IsRunningOnValgrind()) { - // See kernel/fork.c in Linux. There is different ordering of sys_clone - // parameters depending on CONFIG_CLONE_BACKWARDS* configuration options. -#if defined(ARCH_CPU_X86_64) - return syscall(__NR_clone, flags, nullptr, ptid, ctid, nullptr); -#elif defined(ARCH_CPU_X86) || defined(ARCH_CPU_ARM_FAMILY) || \ - defined(ARCH_CPU_MIPS_FAMILY) || defined(ARCH_CPU_MIPS64_FAMILY) - // CONFIG_CLONE_BACKWARDS defined. - return syscall(__NR_clone, flags, nullptr, ptid, nullptr, ctid); -#else -#error "Unsupported architecture" -#endif - } - - jmp_buf env; - if (setjmp(env) == 0) { - return CloneAndLongjmpInChild(flags, ptid, ctid, &env); - } - - return 0; -} - } // namespace base diff --git a/base/process/process_unittest.cc b/base/process/process_unittest.cc index 8130726942a0..1a2af5034ddf 100644 --- a/base/process/process_unittest.cc +++ b/base/process/process_unittest.cc @@ -4,28 +4,13 @@ #include "base/process/process.h" -#include "base/files/file_path.h" -#include "base/files/file_util.h" -#include "base/files/scoped_file.h" -#include "base/posix/eintr_wrapper.h" #include "base/process/kill.h" #include "base/test/multiprocess_test.h" #include "base/test/test_timeouts.h" -#include "base/third_party/dynamic_annotations/dynamic_annotations.h" #include "base/threading/platform_thread.h" -#include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/multiprocess_func_list.h" -#if defined(OS_LINUX) -#include -#include -#include -#include -#include -#include -#endif - namespace { #if defined(OS_WIN) @@ -215,105 +200,4 @@ TEST_F(ProcessTest, SetProcessBackgroundedSelf) { EXPECT_EQ(old_priority, new_priority); } -#if defined(OS_LINUX) -const int kSuccess = 0; - -MULTIPROCESS_TEST_MAIN(CheckPidProcess) { - const pid_t kInitPid = 1; - const pid_t pid = syscall(__NR_getpid); - CHECK(pid == kInitPid); - CHECK(getpid() == pid); - return kSuccess; -} - -TEST_F(ProcessTest, CloneFlags) { - if (RunningOnValgrind() || !PathExists(FilePath("/proc/self/ns/user")) || - !PathExists(FilePath("/proc/self/ns/pid"))) { - // User or PID namespaces are not supported. - return; - } - - LaunchOptions options; - options.clone_flags = CLONE_NEWUSER | CLONE_NEWPID; - - Process process(SpawnChildWithOptions("CheckPidProcess", options)); - ASSERT_TRUE(process.IsValid()); - - int exit_code = 42; - EXPECT_TRUE(process.WaitForExit(&exit_code)); - EXPECT_EQ(kSuccess, exit_code); -} - -TEST(ForkWithFlagsTest, UpdatesPidCache) { - // The libc clone function, which allows ForkWithFlags to keep the pid cache - // up to date, does not work on Valgrind. - if (RunningOnValgrind()) { - return; - } - - // Warm up the libc pid cache, if there is one. - ASSERT_EQ(syscall(__NR_getpid), getpid()); - - pid_t ctid = 0; - const pid_t pid = ForkWithFlags(SIGCHLD | CLONE_CHILD_SETTID, nullptr, &ctid); - if (pid == 0) { - // In child. Check both the raw getpid syscall and the libc getpid wrapper - // (which may rely on a pid cache). - RAW_CHECK(syscall(__NR_getpid) == ctid); - RAW_CHECK(getpid() == ctid); - _exit(kSuccess); - } - - ASSERT_NE(-1, pid); - int status = 42; - ASSERT_EQ(pid, HANDLE_EINTR(waitpid(pid, &status, 0))); - ASSERT_TRUE(WIFEXITED(status)); - EXPECT_EQ(kSuccess, WEXITSTATUS(status)); -} -#endif - -#if defined(OS_POSIX) && !defined(OS_ANDROID) -const char kPipeValue = '\xcc'; - -class ReadFromPipeDelegate : public LaunchOptions::PreExecDelegate { - public: - explicit ReadFromPipeDelegate(int fd) : fd_(fd) {} - ~ReadFromPipeDelegate() override {} - void RunAsyncSafe() override { - char c; - RAW_CHECK(HANDLE_EINTR(read(fd_, &c, 1)) == 1); - RAW_CHECK(IGNORE_EINTR(close(fd_)) == 0); - RAW_CHECK(c == kPipeValue); - } - - private: - int fd_; - DISALLOW_COPY_AND_ASSIGN(ReadFromPipeDelegate); -}; - -TEST_F(ProcessTest, PreExecHook) { - int pipe_fds[2]; - ASSERT_EQ(0, pipe(pipe_fds)); - - ScopedFD read_fd(pipe_fds[0]); - ScopedFD write_fd(pipe_fds[1]); - base::FileHandleMappingVector fds_to_remap; - fds_to_remap.push_back(std::make_pair(read_fd.get(), read_fd.get())); - - ReadFromPipeDelegate read_from_pipe_delegate(read_fd.get()); - LaunchOptions options; - options.fds_to_remap = &fds_to_remap; - options.pre_exec_delegate = &read_from_pipe_delegate; - Process process(SpawnChildWithOptions("SimpleChildProcess", options)); - ASSERT_TRUE(process.IsValid()); - - read_fd.reset(); - ASSERT_EQ(1, HANDLE_EINTR(write(write_fd.get(), &kPipeValue, 1))); - - int exit_code = 42; - EXPECT_TRUE(process.WaitForExit(&exit_code)); - EXPECT_EQ(0, exit_code); -} -#endif // defined(OS_POSIX) && !defined(OS_ANDROID) - } // namespace base diff --git a/base/process/process_util_unittest.cc b/base/process/process_util_unittest.cc index 4b2a575bf7d0..88b4af39765b 100644 --- a/base/process/process_util_unittest.cc +++ b/base/process/process_util_unittest.cc @@ -10,6 +10,8 @@ #include "base/debug/alias.h" #include "base/debug/stack_trace.h" #include "base/files/file_path.h" +#include "base/files/file_util.h" +#include "base/files/scoped_file.h" #include "base/logging.h" #include "base/memory/scoped_ptr.h" #include "base/path_service.h" @@ -27,21 +29,26 @@ #include "base/third_party/dynamic_annotations/dynamic_annotations.h" #include "base/threading/platform_thread.h" #include "base/threading/thread.h" +#include "build/build_config.h" #include "testing/gtest/include/gtest/gtest.h" #include "testing/multiprocess_func_list.h" #if defined(OS_LINUX) #include #include +#include #endif #if defined(OS_POSIX) #include #include #include +#include #include #include #include +#include #include +#include #endif #if defined(OS_WIN) #include @@ -913,4 +920,107 @@ MULTIPROCESS_TEST_MAIN(process_util_test_die_immediately) { return 0; } +#if !defined(OS_ANDROID) +const char kPipeValue = '\xcc'; + +class ReadFromPipeDelegate : public base::LaunchOptions::PreExecDelegate { + public: + explicit ReadFromPipeDelegate(int fd) : fd_(fd) {} + ~ReadFromPipeDelegate() override {} + void RunAsyncSafe() override { + char c; + RAW_CHECK(HANDLE_EINTR(read(fd_, &c, 1)) == 1); + RAW_CHECK(IGNORE_EINTR(close(fd_)) == 0); + RAW_CHECK(c == kPipeValue); + } + + private: + int fd_; + DISALLOW_COPY_AND_ASSIGN(ReadFromPipeDelegate); +}; + +TEST_F(ProcessUtilTest, PreExecHook) { + int pipe_fds[2]; + ASSERT_EQ(0, pipe(pipe_fds)); + + base::ScopedFD read_fd(pipe_fds[0]); + base::ScopedFD write_fd(pipe_fds[1]); + base::FileHandleMappingVector fds_to_remap; + fds_to_remap.push_back(std::make_pair(read_fd.get(), read_fd.get())); + + ReadFromPipeDelegate read_from_pipe_delegate(read_fd.get()); + base::LaunchOptions options; + options.fds_to_remap = &fds_to_remap; + options.pre_exec_delegate = &read_from_pipe_delegate; + base::Process process(SpawnChildWithOptions("SimpleChildProcess", options)); + ASSERT_TRUE(process.IsValid()); + + read_fd.reset(); + ASSERT_EQ(1, HANDLE_EINTR(write(write_fd.get(), &kPipeValue, 1))); + + int exit_code = 42; + EXPECT_TRUE(process.WaitForExit(&exit_code)); + EXPECT_EQ(0, exit_code); +} +#endif // !defined(OS_ANDROID) + #endif // defined(OS_POSIX) + +#if defined(OS_LINUX) +const int kSuccess = 0; + +MULTIPROCESS_TEST_MAIN(CheckPidProcess) { + const pid_t kInitPid = 1; + const pid_t pid = syscall(__NR_getpid); + CHECK(pid == kInitPid); + CHECK(getpid() == pid); + return kSuccess; +} + +TEST_F(ProcessUtilTest, CloneFlags) { + if (RunningOnValgrind() || + !base::PathExists(FilePath("/proc/self/ns/user")) || + !base::PathExists(FilePath("/proc/self/ns/pid"))) { + // User or PID namespaces are not supported. + return; + } + + base::LaunchOptions options; + options.clone_flags = CLONE_NEWUSER | CLONE_NEWPID; + + base::Process process(SpawnChildWithOptions("CheckPidProcess", options)); + ASSERT_TRUE(process.IsValid()); + + int exit_code = 42; + EXPECT_TRUE(process.WaitForExit(&exit_code)); + EXPECT_EQ(kSuccess, exit_code); +} + +TEST(ForkWithFlagsTest, UpdatesPidCache) { + // The libc clone function, which allows ForkWithFlags to keep the pid cache + // up to date, does not work on Valgrind. + if (RunningOnValgrind()) { + return; + } + + // Warm up the libc pid cache, if there is one. + ASSERT_EQ(syscall(__NR_getpid), getpid()); + + pid_t ctid = 0; + const pid_t pid = + base::ForkWithFlags(SIGCHLD | CLONE_CHILD_SETTID, nullptr, &ctid); + if (pid == 0) { + // In child. Check both the raw getpid syscall and the libc getpid wrapper + // (which may rely on a pid cache). + RAW_CHECK(syscall(__NR_getpid) == ctid); + RAW_CHECK(getpid() == ctid); + _exit(kSuccess); + } + + ASSERT_NE(-1, pid); + int status = 42; + ASSERT_EQ(pid, HANDLE_EINTR(waitpid(pid, &status, 0))); + ASSERT_TRUE(WIFEXITED(status)); + EXPECT_EQ(kSuccess, WEXITSTATUS(status)); +} +#endif diff --git a/content/zygote/zygote_linux.cc b/content/zygote/zygote_linux.cc index ba2097ae277f..f7e043ac523f 100644 --- a/content/zygote/zygote_linux.cc +++ b/content/zygote/zygote_linux.cc @@ -22,6 +22,9 @@ #include "base/posix/global_descriptors.h" #include "base/posix/unix_domain_socket_linux.h" #include "base/process/kill.h" +#include "base/process/launch.h" +#include "base/process/process.h" +#include "base/process/process_handle.h" #include "content/common/child_process_sandbox_support_impl_linux.h" #include "content/common/sandbox_linux/sandbox_linux.h" #include "content/common/set_process_title.h" @@ -33,7 +36,6 @@ #include "content/public/common/zygote_fork_delegate_linux.h" #include "ipc/ipc_channel.h" #include "ipc/ipc_switches.h" -#include "sandbox/linux/services/syscall_wrappers.h" #if defined(ADDRESS_SANITIZER) #include diff --git a/sandbox/linux/services/credentials.cc b/sandbox/linux/services/credentials.cc index 274ae2859cd6..291c2cad4450 100644 --- a/sandbox/linux/services/credentials.cc +++ b/sandbox/linux/services/credentials.cc @@ -19,7 +19,7 @@ #include "base/files/file_util.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" -#include "base/process/process.h" +#include "base/process/launch.h" #include "base/template_util.h" #include "base/third_party/valgrind/valgrind.h" #include "sandbox/linux/services/syscall_wrappers.h" -- 2.11.4.GIT