Skip to content

Commit 1eef089

Browse files
Dave Plattandi34
authored andcommitted
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: indent change Bug: 12114500 Change-Id: I090402136a8a8b7d6aad6eb153026e85d7cf6ad3
1 parent acb473a commit 1eef089

File tree

2 files changed

+45
-1
lines changed

2 files changed

+45
-1
lines changed

core/java/com/android/internal/os/ZygoteConnection.java

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -220,9 +220,37 @@ boolean runOnce() throws ZygoteInit.MethodAndArgsCaller {
220220
ZygoteInit.setCloseOnExec(serverPipeFd, true);
221221
}
222222

223+
/**
224+
* In order to avoid leaking descriptors to the Zygote child,
225+
* the native code must close the two Zygote socket descriptors
226+
* in the child process before it switches from Zygote-root to
227+
* the UID and privileges of the application being launched.
228+
*
229+
* In order to avoid "bad file descriptor" errors when the
230+
* two LocalSocket objects are closed, the Posix file
231+
* descriptors are released via a dup2() call which closes
232+
* the socket and substitutes an open descriptor to /dev/null.
233+
*/
234+
235+
int [] fdsToClose = { -1, -1 };
236+
237+
FileDescriptor fd = mSocket.getFileDescriptor();
238+
239+
if (fd != null) {
240+
fdsToClose[0] = fd.getInt$();
241+
}
242+
243+
fd = ZygoteInit.getServerSocketFileDescriptor();
244+
245+
if (fd != null) {
246+
fdsToClose[1] = fd.getInt$();
247+
}
248+
249+
fd = null;
250+
223251
pid = Zygote.forkAndSpecialize(parsedArgs.uid, parsedArgs.gid, parsedArgs.gids,
224252
parsedArgs.debugFlags, rlimits, parsedArgs.mountExternal, parsedArgs.seInfo,
225-
parsedArgs.niceName);
253+
parsedArgs.niceName, fdsToClose);
226254
} catch (IOException ex) {
227255
logAndPrintError(newStderr, "Exception creating pipe", ex);
228256
} catch (ErrnoException ex) {
@@ -875,6 +903,12 @@ private void handleChildProc(Arguments parsedArgs,
875903
FileDescriptor[] descriptors, FileDescriptor pipeFd, PrintStream newStderr)
876904
throws ZygoteInit.MethodAndArgsCaller {
877905

906+
/**
907+
* By the time we get here, the native code has closed the two actual Zygote
908+
* socket connections, and substituted /dev/null in their place. The LocalSocket
909+
* objects still need to be closed properly.
910+
*/
911+
878912
closeSocket();
879913
ZygoteInit.closeServerSocket();
880914

core/java/com/android/internal/os/ZygoteInit.java

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -207,6 +207,16 @@ static void closeServerSocket() {
207207
sServerSocket = null;
208208
}
209209

210+
/**
211+
* Return the server socket's underlying file descriptor, so that
212+
* ZygoteConnection can pass it to the native code for proper
213+
* closure after a child process is forked off.
214+
*/
215+
216+
static FileDescriptor getServerSocketFileDescriptor() {
217+
return sServerSocket.getFileDescriptor();
218+
}
219+
210220
private static final int UNPRIVILEGED_UID = 9999;
211221
private static final int UNPRIVILEGED_GID = 9999;
212222

0 commit comments

Comments
 (0)