Skip to content

Commit

Permalink
Finish fixing Zygote descriptor leakage problem
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Dave Platt authored and andi34 committed Jun 6, 2016
1 parent 36e356c commit 7004b0c
Showing 1 changed file with 76 additions and 3 deletions.
79 changes: 76 additions & 3 deletions vm/native/dalvik_system_Zygote.cpp
Expand Up @@ -28,6 +28,8 @@
#include <grp.h>
#include <errno.h>
#include <paths.h>
#include <fcntl.h>
#include <unistd.h>
#include <sys/personality.h>
#include <sys/stat.h>
#include <sys/mount.h>
Expand Down Expand Up @@ -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;

Expand All @@ -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;
Expand Down Expand Up @@ -528,6 +570,9 @@ static pid_t forkAndSpecializeCommon(const u4* args, bool isSystemServer)
dvmAbort();
}
}
if (!legacyFork) {
fdsToClose = (ArrayObject *)args[8];
}
}

if (!gDvm.zygote) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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,
Expand All @@ -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);
}
Expand All @@ -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) {
Expand All @@ -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",
Expand Down

0 comments on commit 7004b0c

Please sign in to comment.