From 7004b0c9e811582c368f6669a79a283eb8ad6b5b Mon Sep 17 00:00:00 2001 From: Dave Platt Date: Wed, 5 Feb 2014 17:03:52 -0800 Subject: [PATCH] Finish fixing Zygote descriptor leakage problem In order to prevent Zygote descriptors from leaking into the child environment, they should be closed by the forked-off child process before the child switches to the application UID. These changes close the descriptors via dup2(), substituting a descriptor open to /dev/null in their place; this allows the Zygote Java code to close the FileDescriptor objects cleanly. This is a multi-project change: dalvik, art, libcore, frameworks/base, and external/sepolicy are affected. The CLs need to be approved together, lest the build break or the software fail to boot. Round 2: whitespace Bug: 12114500 Change-Id: I989c83291d0c42d4cad63f24a3e98a93e231c9d3 --- vm/native/dalvik_system_Zygote.cpp | 79 ++++++++++++++++++++++++++++-- 1 file changed, 76 insertions(+), 3 deletions(-) diff --git a/vm/native/dalvik_system_Zygote.cpp b/vm/native/dalvik_system_Zygote.cpp index e2b618bab..15f4eda27 100644 --- a/vm/native/dalvik_system_Zygote.cpp +++ b/vm/native/dalvik_system_Zygote.cpp @@ -28,6 +28,8 @@ #include #include #include +#include +#include #include #include #include @@ -482,10 +484,49 @@ static bool needsNoRandomizeWorkaround() { #endif } +#ifdef HAVE_ANDROID_OS + +// Utility to close down the Zygote socket file descriptors while +// the child is still running as root with Zygote's privileges. Each +// descriptor (if any) is closed via dup2(), replacing it with a valid +// (open) descriptor to /dev/null. + +static void detachDescriptors(ArrayObject* fdsToClose) { + if (!fdsToClose) { + return; + } + size_t count = fdsToClose->length; + int *ar = (int *) (void *) fdsToClose->contents; + if (!ar) { + ALOG(LOG_ERROR, ZYGOTE_LOG_TAG, "Bad fd array"); + dvmAbort(); + } + size_t i; + int devnull; + for (i = 0; i < count; i++) { + if (ar[1] < 0) { + continue; + } + devnull = open("/dev/null", O_RDWR); + if (devnull < 0) { + ALOG(LOG_ERROR, ZYGOTE_LOG_TAG, "Failed to open /dev/null"); + dvmAbort(); + } + ALOG(LOG_VERBOSE, ZYGOTE_LOG_TAG, "Switching descriptor %d to /dev/null", ar[i]); + if (dup2(devnull, ar[i]) < 0) { + ALOG(LOG_ERROR, ZYGOTE_LOG_TAG, "Failed dup2() on descriptor %d", ar[i]); + dvmAbort(); + } + close(devnull); + } +} + +#endif + /* * Utility routine to fork zygote and specialize the child process. */ -static pid_t forkAndSpecializeCommon(const u4* args, bool isSystemServer) +static pid_t forkAndSpecializeCommon(const u4* args, bool isSystemServer, bool legacyFork) { pid_t pid; @@ -494,6 +535,7 @@ static pid_t forkAndSpecializeCommon(const u4* args, bool isSystemServer) ArrayObject* gids = (ArrayObject *)args[2]; u4 debugFlags = args[3]; ArrayObject *rlimits = (ArrayObject *)args[4]; + ArrayObject *fdsToClose = NULL; u4 mountMode = MOUNT_EXTERNAL_NONE; int64_t permittedCapabilities, effectiveCapabilities; char *seInfo = NULL; @@ -528,6 +570,9 @@ static pid_t forkAndSpecializeCommon(const u4* args, bool isSystemServer) dvmAbort(); } } + if (!legacyFork) { + fdsToClose = (ArrayObject *)args[8]; + } } if (!gDvm.zygote) { @@ -555,6 +600,10 @@ static pid_t forkAndSpecializeCommon(const u4* args, bool isSystemServer) extern int gMallocLeakZygoteChild; gMallocLeakZygoteChild = 1; + // Unhook from the Zygote sockets immediately + + detachDescriptors(fdsToClose); + /* keep caps across UID change, unless we're staying root */ if (uid != 0) { err = prctl(PR_SET_KEEPCAPS, 1, 0, 0, 0); @@ -679,6 +728,28 @@ static pid_t forkAndSpecializeCommon(const u4* args, bool isSystemServer) return pid; } +/* + * We must expose both flavors of the native forking code for a while, + * as this Dalvik change must be reviewed and checked in before the + * libcore and frameworks changes. The legacy fork API function and + * registration can be removed later. + */ + +/* + * native public static int nativeForkAndSpecialize(int uid, int gid, + * int[] gids, int debugFlags, int[][] rlimits, int mountExternal, + * String seInfo, String niceName, int[] fdsToClose); + */ +static void Dalvik_dalvik_system_Zygote_forkAndSpecialize_new(const u4* args, + JValue* pResult) +{ + pid_t pid; + + pid = forkAndSpecializeCommon(args, false, false); + + RETURN_INT(pid); +} + /* * native public static int nativeForkAndSpecialize(int uid, int gid, * int[] gids, int debugFlags, int[][] rlimits, int mountExternal, @@ -689,7 +760,7 @@ static void Dalvik_dalvik_system_Zygote_forkAndSpecialize(const u4* args, { pid_t pid; - pid = forkAndSpecializeCommon(args, false); + pid = forkAndSpecializeCommon(args, false, true); RETURN_INT(pid); } @@ -703,7 +774,7 @@ static void Dalvik_dalvik_system_Zygote_forkSystemServer( const u4* args, JValue* pResult) { pid_t pid; - pid = forkAndSpecializeCommon(args, true); + pid = forkAndSpecializeCommon(args, true, true); /* The zygote process checks whether the child process has died or not. */ if (pid > 0) { @@ -726,6 +797,8 @@ static void Dalvik_dalvik_system_Zygote_forkSystemServer( const DalvikNativeMethod dvm_dalvik_system_Zygote[] = { { "nativeFork", "()I", Dalvik_dalvik_system_Zygote_fork }, + { "nativeForkAndSpecialize", "(II[II[[IILjava/lang/String;Ljava/lang/String;[I)I", + Dalvik_dalvik_system_Zygote_forkAndSpecialize_new }, { "nativeForkAndSpecialize", "(II[II[[IILjava/lang/String;Ljava/lang/String;)I", Dalvik_dalvik_system_Zygote_forkAndSpecialize }, { "nativeForkSystemServer", "(II[II[[IJJ)I",