From 4bfdcb39329f778c46f73827f213c80b93c4e7d3 Mon Sep 17 00:00:00 2001
From: Joe Tanen <joe.tanen@twosixlabs.com>
Date: Wed, 25 Oct 2017 08:07:26 -0400
Subject: [PATCH] init: fixed issues related to forking services

Fixed issues related to forking services into new PID + mount
namespaces.

Remounting rootfs recursively as slave when creating a service in new
PID + mount namespaces. This prevents the service from interfering with
mount points in the parent namespace.

Unmount then mount /proc instead of mounting it with MS_REMOUNT, since
MS_REMOUNT is not sufficient to update /proc to the state appropriate
for the new PID namespace. Note that the /proc mount options specified
here are not the same as those used in the default mount namespace. I
kept them consistent with those used in the code prior to this fix.

Test: Used custom sleepd service to test init 'namespace' keyword.
Tested on angler in oreo-dev - I had to add PID namespaces to the
kernel (commit ad82c662).

Change-Id: I859104525f82fef3400d5abbad465331fc3d732f
---
 init/service.cpp | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

diff --git a/init/service.cpp b/init/service.cpp
index 765b61e56..bcebb4989 100644
--- a/init/service.cpp
+++ b/init/service.cpp
@@ -101,8 +101,22 @@ static void SetUpPidNamespace(const std::string& service_name) {
 
     // It's OK to LOG(FATAL) in this function since it's running in the first
     // child process.
-    if (mount("", "/proc", "proc", kSafeFlags | MS_REMOUNT, "") == -1) {
-        PLOG(FATAL) << "couldn't remount(/proc) for " << service_name;
+
+    // Recursively remount / as slave like zygote does so unmounting and mounting /proc
+    // doesn't interfere with the parent namespace's /proc mount. This will also
+    // prevent any other mounts/unmounts initiated by the service from interfering
+    // with the parent namespace but will still allow mount events from the parent
+    // namespace to propagate to the child.
+    if (mount("rootfs", "/", nullptr, (MS_SLAVE | MS_REC), nullptr) == -1) {
+        PLOG(FATAL) << "couldn't remount(/) recursively as slave for " << service_name;
+    }
+    // umount() then mount() /proc.
+    // Note that it is not sufficient to mount with MS_REMOUNT.
+    if (umount("/proc") == -1) {
+        PLOG(FATAL) << "couldn't umount(/proc) for " << service_name;
+    }
+    if (mount("", "/proc", "proc", kSafeFlags, "") == -1) {
+        PLOG(FATAL) << "couldn't mount(/proc) for " << service_name;
     }
 
     if (prctl(PR_SET_NAME, service_name.c_str()) == -1) {