From fa9e16efaf0e885f6044e725eb759ef6de10f7ef Mon Sep 17 00:00:00 2001
From: Elliott Hughes <enh@google.com>
Date: Mon, 23 Jun 2014 17:49:45 -0700
Subject: [PATCH] Fix getpid caching across a clone.

If you make clone, fork, or vfork system calls directly, you're still
on your own, but we now do the right thing for the clone wrapper.
With this implementation, children lose the getpid caching, but we've
no reason to think that that covers any significant use cases.

Bug: 15387103
Change-Id: Icfab6b63c708fea830960742ec92aeba8ce7680d
---
 libc/bionic/clone.cpp | 17 ++++++++++++++++-
 tests/unistd_test.cpp | 37 +++++++++++++++++++++++++++++++------
 2 files changed, 47 insertions(+), 7 deletions(-)

diff --git a/libc/bionic/clone.cpp b/libc/bionic/clone.cpp
index 001e245d7..0a0fdd526 100644
--- a/libc/bionic/clone.cpp
+++ b/libc/bionic/clone.cpp
@@ -31,6 +31,8 @@
 #include <stdlib.h>
 #include <stdarg.h>
 
+#include "pthread_internal.h"
+
 extern "C" pid_t __bionic_clone(uint32_t flags, void* child_stack, int* parent_tid, void* tls, int* child_tid, int (*fn)(void*), void* arg);
 extern "C" __noreturn void __exit(int status);
 
@@ -64,5 +66,18 @@ int clone(int (*fn)(void*), void* child_stack, int flags, void* arg, ...) {
   child_stack_addr &= ~0xf;
   child_stack = reinterpret_cast<void*>(child_stack_addr);
 
-  return __bionic_clone(flags, child_stack, parent_tid, new_tls, child_tid, fn, arg);
+  // Remember the parent pid and invalidate the cached value while we clone.
+  pthread_internal_t* self = __get_thread();
+  pid_t parent_pid = self->invalidate_cached_pid();
+
+  // Actually do the clone.
+  int clone_result = __bionic_clone(flags, child_stack, parent_tid, new_tls, child_tid, fn, arg);
+
+  // We're the parent, so put our known pid back in place.
+  // We leave the child without a cached pid, but:
+  // 1. pthread_create gives its children their own pthread_internal_t with the correct pid.
+  // 2. fork makes a clone system call directly.
+  // If any other cases become important, we could use a double trampoline like __pthread_start.
+  self->set_cached_pid(parent_pid);
+  return clone_result;
 }
diff --git a/tests/unistd_test.cpp b/tests/unistd_test.cpp
index 95e63b368..58c9ad94f 100644
--- a/tests/unistd_test.cpp
+++ b/tests/unistd_test.cpp
@@ -381,6 +381,14 @@ TEST(unistd, fsync) {
   TestFsyncFunction(fsync);
 }
 
+static void AssertGetPidCorrect() {
+  // The loop is just to make manual testing/debugging with strace easier.
+  pid_t getpid_syscall_result = syscall(__NR_getpid);
+  for (size_t i = 0; i < 128; ++i) {
+    ASSERT_EQ(getpid_syscall_result, getpid());
+  }
+}
+
 TEST(unistd, getpid_caching_and_fork) {
   pid_t parent_pid = getpid();
   ASSERT_EQ(syscall(__NR_getpid), parent_pid);
@@ -389,7 +397,7 @@ TEST(unistd, getpid_caching_and_fork) {
   ASSERT_NE(fork_result, -1);
   if (fork_result == 0) {
     // We're the child.
-    ASSERT_EQ(syscall(__NR_getpid), getpid());
+    AssertGetPidCorrect();
     ASSERT_EQ(parent_pid, getppid());
     _exit(123);
   } else {
@@ -403,12 +411,29 @@ TEST(unistd, getpid_caching_and_fork) {
   }
 }
 
-static void GetPidCachingHelperHelper() {
-  ASSERT_EQ(syscall(__NR_getpid), getpid());
+static int GetPidCachingCloneStartRoutine(void*) {
+  AssertGetPidCorrect();
+  return 123;
 }
 
-static void* GetPidCachingHelper(void*) {
-  GetPidCachingHelperHelper(); // Can't assert in a non-void function.
+TEST(unistd, getpid_caching_and_clone) {
+  pid_t parent_pid = getpid();
+  ASSERT_EQ(syscall(__NR_getpid), parent_pid);
+
+  void* child_stack[1024];
+  int clone_result = clone(GetPidCachingCloneStartRoutine, &child_stack[1024], CLONE_NEWNS | SIGCHLD, NULL);
+  ASSERT_NE(clone_result, -1);
+
+  ASSERT_EQ(parent_pid, getpid());
+
+  int status;
+  ASSERT_EQ(clone_result, waitpid(clone_result, &status, 0));
+  ASSERT_TRUE(WIFEXITED(status));
+  ASSERT_EQ(123, WEXITSTATUS(status));
+}
+
+static void* GetPidCachingPthreadStartRoutine(void*) {
+  AssertGetPidCorrect();
   return NULL;
 }
 
@@ -416,7 +441,7 @@ TEST(unistd, getpid_caching_and_pthread_create) {
   pid_t parent_pid = getpid();
 
   pthread_t t;
-  ASSERT_EQ(0, pthread_create(&t, NULL, GetPidCachingHelper, NULL));
+  ASSERT_EQ(0, pthread_create(&t, NULL, GetPidCachingPthreadStartRoutine, NULL));
 
   ASSERT_EQ(parent_pid, getpid());