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: indent change

Bug: 12114500
Change-Id: I090402136a8a8b7d6aad6eb153026e85d7cf6ad3
  • Loading branch information
Dave Platt authored and hyperb1iss committed Feb 21, 2014
1 parent 5166d8e commit fc2fb1d
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 1 deletion.
36 changes: 35 additions & 1 deletion core/java/com/android/internal/os/ZygoteConnection.java
Expand Up @@ -220,9 +220,37 @@ boolean runOnce() throws ZygoteInit.MethodAndArgsCaller {
ZygoteInit.setCloseOnExec(serverPipeFd, true);
}

/**
* In order to avoid leaking descriptors to the Zygote child,
* the native code must close the two Zygote socket descriptors
* in the child process before it switches from Zygote-root to
* the UID and privileges of the application being launched.
*
* In order to avoid "bad file descriptor" errors when the
* two LocalSocket objects are closed, the Posix file
* descriptors are released via a dup2() call which closes
* the socket and substitutes an open descriptor to /dev/null.
*/

int [] fdsToClose = { -1, -1 };

FileDescriptor fd = mSocket.getFileDescriptor();

if (fd != null) {
fdsToClose[0] = fd.getInt$();
}

fd = ZygoteInit.getServerSocketFileDescriptor();

if (fd != null) {
fdsToClose[1] = fd.getInt$();
}

fd = null;

pid = Zygote.forkAndSpecialize(parsedArgs.uid, parsedArgs.gid, parsedArgs.gids,
parsedArgs.debugFlags, rlimits, parsedArgs.mountExternal, parsedArgs.seInfo,
parsedArgs.niceName);
parsedArgs.niceName, fdsToClose);
} catch (IOException ex) {
logAndPrintError(newStderr, "Exception creating pipe", ex);
} catch (ErrnoException ex) {
Expand Down Expand Up @@ -875,6 +903,12 @@ private void handleChildProc(Arguments parsedArgs,
FileDescriptor[] descriptors, FileDescriptor pipeFd, PrintStream newStderr)
throws ZygoteInit.MethodAndArgsCaller {

/**
* By the time we get here, the native code has closed the two actual Zygote
* socket connections, and substituted /dev/null in their place. The LocalSocket
* objects still need to be closed properly.
*/

closeSocket();
ZygoteInit.closeServerSocket();

Expand Down
10 changes: 10 additions & 0 deletions core/java/com/android/internal/os/ZygoteInit.java
Expand Up @@ -201,6 +201,16 @@ static void closeServerSocket() {
sServerSocket = null;
}

/**
* Return the server socket's underlying file descriptor, so that
* ZygoteConnection can pass it to the native code for proper
* closure after a child process is forked off.
*/

static FileDescriptor getServerSocketFileDescriptor() {
return sServerSocket.getFileDescriptor();
}

private static final int UNPRIVILEGED_UID = 9999;
private static final int UNPRIVILEGED_GID = 9999;

Expand Down

0 comments on commit fc2fb1d

Please sign in to comment.