Skip to content

Commit 479fe13

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: use proper type (jsize) for iteration variable; whitespace. Round 3: indentation. Bug: 12114500 Change-Id: I31d25831b477b076d980c531d57773f9c8c83261
1 parent d5e4ac0 commit 479fe13

File tree

1 file changed

+51
-5
lines changed

1 file changed

+51
-5
lines changed

runtime/native/dalvik_system_Zygote.cc

Lines changed: 51 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,8 +23,10 @@
2323
#include <signal.h>
2424
#include <stdlib.h>
2525
#include <sys/types.h>
26+
#include <sys/stat.h>
2627
#include <sys/wait.h>
2728
#include <unistd.h>
29+
#include <fcntl.h>
2830

2931
#include "cutils/fs.h"
3032
#include "cutils/multiuser.h"
@@ -401,12 +403,43 @@ static bool NeedsNoRandomizeWorkaround() {
401403
}
402404
#endif
403405

406+
// Utility to close down the Zygote socket file descriptors while
407+
// the child is still running as root with Zygote's privileges. Each
408+
// descriptor (if any) is closed via dup2(), replacing it with a valid
409+
// (open) descriptor to /dev/null.
410+
411+
static void DetachDescriptors(JNIEnv* env, jintArray fdsToClose) {
412+
if (!fdsToClose) {
413+
return;
414+
}
415+
jsize count = env->GetArrayLength(fdsToClose);
416+
jint *ar = env->GetIntArrayElements(fdsToClose, 0);
417+
if (!ar) {
418+
PLOG(FATAL) << "Bad fd array";
419+
}
420+
jsize i;
421+
int devnull;
422+
for (i = 0; i < count; i++) {
423+
devnull = open("/dev/null", O_RDWR);
424+
if (devnull < 0) {
425+
PLOG(FATAL) << "Failed to open /dev/null";
426+
continue;
427+
}
428+
PLOG(VERBOSE) << "Switching descriptor " << ar[i] << " to /dev/null";
429+
if (dup2(devnull, ar[i]) < 0) {
430+
PLOG(FATAL) << "Failed dup2() on descriptor " << ar[i];
431+
}
432+
close(devnull);
433+
}
434+
}
435+
404436
// Utility routine to fork zygote and specialize the child process.
405437
static pid_t ForkAndSpecializeCommon(JNIEnv* env, uid_t uid, gid_t gid, jintArray javaGids,
406438
jint debug_flags, jobjectArray javaRlimits,
407439
jlong permittedCapabilities, jlong effectiveCapabilities,
408440
jint mount_external,
409-
jstring java_se_info, jstring java_se_name, bool is_system_server) {
441+
jstring java_se_info, jstring java_se_name,
442+
bool is_system_server, jintArray fdsToClose) {
410443
Runtime* runtime = Runtime::Current();
411444
CHECK(runtime->IsZygote()) << "runtime instance not started with -Xzygote";
412445
if (!runtime->PreZygoteFork()) {
@@ -425,6 +458,9 @@ static pid_t ForkAndSpecializeCommon(JNIEnv* env, uid_t uid, gid_t gid, jintArra
425458
// The child process.
426459
gMallocLeakZygoteChild = 1;
427460

461+
// Clean up any descriptors which must be closed immediately
462+
DetachDescriptors(env, fdsToClose);
463+
428464
// Keep capabilities across UID change, unless we're staying root.
429465
if (uid != 0) {
430466
EnableKeepCapabilities();
@@ -516,10 +552,19 @@ static pid_t ForkAndSpecializeCommon(JNIEnv* env, uid_t uid, gid_t gid, jintArra
516552
return pid;
517553
}
518554

555+
static jint Zygote_nativeForkAndSpecialize_new(JNIEnv* env, jclass, jint uid, jint gid, jintArray gids,
556+
jint debug_flags, jobjectArray rlimits,
557+
jint mount_external, jstring se_info, jstring se_name,
558+
jintArray fdsToClose) {
559+
return ForkAndSpecializeCommon(env, uid, gid, gids, debug_flags, rlimits, 0, 0, mount_external,
560+
se_info, se_name, false, fdsToClose);
561+
}
562+
519563
static jint Zygote_nativeForkAndSpecialize(JNIEnv* env, jclass, jint uid, jint gid, jintArray gids,
520-
jint debug_flags, jobjectArray rlimits, jint mount_external,
521-
jstring se_info, jstring se_name) {
522-
return ForkAndSpecializeCommon(env, uid, gid, gids, debug_flags, rlimits, 0, 0, mount_external, se_info, se_name, false);
564+
jint debug_flags, jobjectArray rlimits,
565+
jint mount_external, jstring se_info, jstring se_name) {
566+
return ForkAndSpecializeCommon(env, uid, gid, gids, debug_flags, rlimits, 0, 0, mount_external,
567+
se_info, se_name, false, NULL);
523568
}
524569

525570
static jint Zygote_nativeForkSystemServer(JNIEnv* env, jclass, uid_t uid, gid_t gid, jintArray gids,
@@ -528,7 +573,7 @@ static jint Zygote_nativeForkSystemServer(JNIEnv* env, jclass, uid_t uid, gid_t
528573
pid_t pid = ForkAndSpecializeCommon(env, uid, gid, gids,
529574
debug_flags, rlimits,
530575
permittedCapabilities, effectiveCapabilities,
531-
MOUNT_EXTERNAL_NONE, NULL, NULL, true);
576+
MOUNT_EXTERNAL_NONE, NULL, NULL, true, NULL);
532577
if (pid > 0) {
533578
// The zygote process checks whether the child process has died or not.
534579
LOG(INFO) << "System server process " << pid << " has been created";
@@ -545,6 +590,7 @@ static jint Zygote_nativeForkSystemServer(JNIEnv* env, jclass, uid_t uid, gid_t
545590
}
546591

547592
static JNINativeMethod gMethods[] = {
593+
NATIVE_METHOD(Zygote, nativeForkAndSpecialize_new, "(II[II[[IILjava/lang/String;Ljava/lang/String;[I)I"),
548594
NATIVE_METHOD(Zygote, nativeForkAndSpecialize, "(II[II[[IILjava/lang/String;Ljava/lang/String;)I"),
549595
NATIVE_METHOD(Zygote, nativeForkSystemServer, "(II[II[[IJJ)I"),
550596
};

0 commit comments

Comments
 (0)