Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(459)

Unified Diff: sandbox/linux/services/syscall_wrappers.cc

Issue 800183004: Add a ForkWithFlags wrapper using the libc clone wrapper. (Closed) Base URL: https://chromium.googlesource.com/chromium/src.git@master
Patch Set: Created 6 years ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: sandbox/linux/services/syscall_wrappers.cc
diff --git a/sandbox/linux/services/syscall_wrappers.cc b/sandbox/linux/services/syscall_wrappers.cc
index c3685197e88c7313e713c803a962b8e2f920582c..1fcdd758a7ac564b86fd78140cbff886fe4c2aef 100644
--- a/sandbox/linux/services/syscall_wrappers.cc
+++ b/sandbox/linux/services/syscall_wrappers.cc
@@ -4,13 +4,18 @@
#include "sandbox/linux/services/syscall_wrappers.h"
+#include <pthread.h>
+#include <sched.h>
+#include <setjmp.h>
#include <sys/resource.h>
#include <sys/syscall.h>
#include <sys/time.h>
#include <sys/types.h>
#include <unistd.h>
+#include "base/compiler_specific.h"
#include "base/logging.h"
+#include "base/third_party/valgrind/valgrind.h"
#include "build/build_config.h"
#include "sandbox/linux/services/linux_syscalls.h"
@@ -24,27 +29,34 @@ pid_t sys_gettid(void) {
return syscall(__NR_gettid);
}
-long sys_clone(unsigned long flags) {
- return sys_clone(flags, nullptr, nullptr, nullptr, nullptr);
+namespace {
+
+bool CloneArgumentsValid(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;
+
+ return !(clone_tls_used || invalid_ctid || invalid_ptid || clone_vm_used);
}
+} // namespace
+
long sys_clone(unsigned long flags,
- void* child_stack,
+ decltype(nullptr) child_stack,
pid_t* ptid,
pid_t* ctid,
decltype(nullptr) tls) {
- 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;
- const bool invalid_stack = (flags & CLONE_VM) && !child_stack;
- if (clone_tls_used || invalid_ctid || invalid_ptid || invalid_stack) {
+ if (!CloneArgumentsValid(flags, ptid, ctid)) {
RAW_LOG(FATAL, "Invalid usage of sys_clone");
}
-// See kernel/fork.c in Linux. There is different ordering of sys_clone
-// parameters depending on CONFIG_CLONE_BACKWARDS* configuration options.
+ // 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, child_stack, ptid, ctid, tls);
#elif defined(ARCH_CPU_X86) || defined(ARCH_CPU_ARM_FAMILY) || \
@@ -54,6 +66,74 @@ long sys_clone(unsigned long flags,
#endif
}
+long sys_clone(unsigned long flags) {
+ return sys_clone(flags, nullptr, nullptr, nullptr, nullptr);
+}
+
+namespace {
jln (very slow on Chromium) 2014/12/17 02:23:42 It's not typical to have multiple anonymous namesp
rickyz (no longer on Chrome) 2014/12/17 02:31:34 Done.
+
+bool IsRunningOnValgrind() {
+ return RUNNING_ON_VALGRIND;
+}
+
+int CloneHelper(void* arg) {
+ jmp_buf* env_ptr = reinterpret_cast<jmp_buf*>(arg);
+ // This function runs on the stack specified on the clone call. Use longjmp to
+ // switch back to the original stack so the child can return from sys_clone.
jln (very slow on Chromium) 2014/12/17 02:23:42 Nit: let's move this comment above the function de
rickyz (no longer on Chrome) 2014/12/17 02:31:34 Done.
+ 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.
jln (very slow on Chromium) 2014/12/17 02:23:42 Nit: add a https://crbug.com/ link
rickyz (no longer on Chrome) 2014/12/17 02:31:34 Done.
+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
+
+pid_t ForkWithFlags(unsigned long flags, pid_t* ptid, pid_t* ctid) {
+ // 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()) {
+ return sys_clone(flags, nullptr, ptid, ctid, nullptr);
+ }
+
+ if (!CloneArgumentsValid(flags, ptid, ctid)) {
jln (very slow on Chromium) 2014/12/17 02:23:42 This should be above, no? (And run also on Valgrin
rickyz (no longer on Chrome) 2014/12/17 02:31:34 Done.
rickyz (no longer on Chrome) 2014/12/17 02:31:34 Oops, I had left it below since sys_clone has the
+ RAW_LOG(FATAL, "Invalid usage of ForkWithFlags");
+ }
+
+ jmp_buf env;
+ if (setjmp(env) == 0) {
+ return CloneAndLongjmpInChild(flags, ptid, ctid, &env);
+ }
+
+ return 0;
+}
+
void sys_exit_group(int status) {
syscall(__NR_exit_group, status);
}

Powered by Google App Engine
This is Rietveld 408576698