Android/frameworks_base
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Merge "Fixes two data races in USAP pool management." into qt-dev

Browse Source
This commit is contained in:
Christian Wailes
2019-06-10 22:09:43 +00:00
committed by Android (Google) Code Review
parent 8e13ea659e f59ea2cc39
commit 5e468cf4dd
3 changed files with 139 additions and 74 deletions
Download Patch File Download Diff File Expand all files Collapse all files

158
core/java/com/android/internal/os/Zygote.java
View File

@@ -519,6 +519,9 @@ public final class Zygote {
try {
sessionSocket = usapPoolSocket.accept();
// Block SIGTERM so we won't be killed if the Zygote flushes the USAP pool.
blockSigTerm();
BufferedReader usapReader =
new BufferedReader(new InputStreamReader(sessionSocket.getInputStream()));
usapOutputStream =
@@ -537,87 +540,116 @@ public final class Zygote {
} else {
Log.e("USAP", "Truncated command received.");
IoUtils.closeQuietly(sessionSocket);
// Re-enable SIGTERM so the USAP can be flushed from the pool if necessary.
unblockSigTerm();
}
} catch (Exception ex) {
Log.e("USAP", ex.getMessage());
IoUtils.closeQuietly(sessionSocket);
// Re-enable SIGTERM so the USAP can be flushed from the pool if necessary.
unblockSigTerm();
}
}
applyUidSecurityPolicy(args, peerCredentials);
applyDebuggerSystemProperty(args);
int[][] rlimits = null;
if (args.mRLimits != null) {
rlimits = args.mRLimits.toArray(INT_ARRAY_2D);
}
// This must happen before the SELinux policy for this process is
// changed when specializing.
try {
// Used by ZygoteProcess.zygoteSendArgsAndGetResult to fill in a
// Process.ProcessStartResult object.
usapOutputStream.writeInt(pid);
} catch (IOException ioEx) {
Log.e("USAP", "Failed to write response to session socket: " + ioEx.getMessage());
System.exit(-1);
} finally {
IoUtils.closeQuietly(sessionSocket);
// SIGTERM is blocked on loop exit. This prevents a USAP that is specializing from
// being killed during a pool flush.
applyUidSecurityPolicy(args, peerCredentials);
applyDebuggerSystemProperty(args);
int[][] rlimits = null;
if (args.mRLimits != null) {
rlimits = args.mRLimits.toArray(INT_ARRAY_2D);
}
// This must happen before the SELinux policy for this process is
// changed when specializing.
try {
// Used by ZygoteProcess.zygoteSendArgsAndGetResult to fill in a
// Process.ProcessStartResult object.
usapOutputStream.writeInt(pid);
} catch (IOException ioEx) {
Log.e("USAP", "Failed to write response to session socket: "
+ ioEx.getMessage());
throw new RuntimeException(ioEx);
} finally {
IoUtils.closeQuietly(sessionSocket);
try {
// This socket is closed using Os.close due to an issue with the implementation
// of LocalSocketImp.close(). Because the raw FD is created by init and then
// loaded from an environment variable (as opposed to being created by the
// LocalSocketImpl itself) the current implementation will not actually close
// the underlying FD.
//
// See b/130309968 for discussion of this issue.
Os.close(usapPoolSocket.getFileDescriptor());
} catch (ErrnoException ex) {
Log.e("USAP", "Failed to close USAP pool socket");
throw new RuntimeException(ex);
}
}
try {
// This socket is closed using Os.close due to an issue with the implementation of
// LocalSocketImp.close. Because the raw FD is created by init and then loaded from
// an environment variable (as opposed to being created by the LocalSocketImpl
// itself) the current implementation will not actually close the underlying FD.
//
// See b/130309968 for discussion of this issue.
Os.close(usapPoolSocket.getFileDescriptor());
} catch (ErrnoException ex) {
Log.e("USAP", "Failed to close USAP pool socket: " + ex.getMessage());
ByteArrayOutputStream buffer =
new ByteArrayOutputStream(Zygote.USAP_MANAGEMENT_MESSAGE_BYTES);
DataOutputStream outputStream = new DataOutputStream(buffer);
// This is written as a long so that the USAP reporting pipe and USAP pool event FD
// handlers in ZygoteServer.runSelectLoop can be unified. These two cases should
// both send/receive 8 bytes.
outputStream.writeLong(pid);
outputStream.flush();
Os.write(writePipe, buffer.toByteArray(), 0, buffer.size());
} catch (Exception ex) {
Log.e("USAP",
String.format("Failed to write PID (%d) to pipe (%d): %s",
pid, writePipe.getInt$(), ex.getMessage()));
throw new RuntimeException(ex);
} finally {
IoUtils.closeQuietly(writePipe);
}
}
try {
ByteArrayOutputStream buffer =
new ByteArrayOutputStream(Zygote.USAP_MANAGEMENT_MESSAGE_BYTES);
DataOutputStream outputStream = new DataOutputStream(buffer);
specializeAppProcess(args.mUid, args.mGid, args.mGids,
args.mRuntimeFlags, rlimits, args.mMountExternal,
args.mSeInfo, args.mNiceName, args.mStartChildZygote,
args.mInstructionSet, args.mAppDataDir);
// This is written as a long so that the USAP reporting pipe and USAP pool event FD
// handlers in ZygoteServer.runSelectLoop can be unified. These two cases should both
// send/receive 8 bytes.
outputStream.writeLong(pid);
outputStream.flush();
disableExecuteOnly(args.mTargetSdkVersion);
Os.write(writePipe, buffer.toByteArray(), 0, buffer.size());
} catch (Exception ex) {
Log.e("USAP",
String.format("Failed to write PID (%d) to pipe (%d): %s",
pid, writePipe.getInt$(), ex.getMessage()));
System.exit(-1);
if (args.mNiceName != null) {
Process.setArgV0(args.mNiceName);
}
// End of the postFork event.
Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);
return ZygoteInit.zygoteInit(args.mTargetSdkVersion,
args.mRemainingArgs,
null /* classLoader */);
} finally {
IoUtils.closeQuietly(writePipe);
// Unblock SIGTERM to restore the process to default behavior.
unblockSigTerm();
}
specializeAppProcess(args.mUid, args.mGid, args.mGids,
args.mRuntimeFlags, rlimits, args.mMountExternal,
args.mSeInfo, args.mNiceName, args.mStartChildZygote,
args.mInstructionSet, args.mAppDataDir);
disableExecuteOnly(args.mTargetSdkVersion);
if (args.mNiceName != null) {
Process.setArgV0(args.mNiceName);
}
// End of the postFork event.
Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);
return ZygoteInit.zygoteInit(args.mTargetSdkVersion,
args.mRemainingArgs,
null /* classLoader */);
}
private static void blockSigTerm() {
nativeBlockSigTerm();
}
private static native void nativeBlockSigTerm();
private static void unblockSigTerm() {
nativeUnblockSigTerm();
}
private static native void nativeUnblockSigTerm();
private static final String USAP_ERROR_PREFIX = "Invalid command to USAP: ";
/**

3
core/java/com/android/internal/os/ZygoteConnection.java
View File

@@ -338,6 +338,7 @@ class ZygoteConnection {
Runnable stateChangeCode) {
try {
if (zygoteServer.isUsapPoolEnabled()) {
Log.i(TAG, "Emptying USAP Pool due to state change.");
Zygote.emptyUsapPool();
}
@@ -351,6 +352,8 @@ class ZygoteConnection {
if (fpResult != null) {
zygoteServer.setForkChild();
return fpResult;
} else {
Log.i(TAG, "Finished refilling USAP Pool after state change.");
}
}

52
core/jni/com_android_internal_os_Zygote.cpp
View File

@@ -198,7 +198,7 @@ class UsapTableEntry {
* PIDs don't match nothing will happen.
*
* @param pid The ID of the process who's entry we want to clear.
* @return True if the entry was cleared; false otherwise
* @return True if the entry was cleared by this call; false otherwise
*/
bool ClearForPID(int32_t pid) {
EntryStorage storage = mStorage.load();
@@ -212,14 +212,16 @@ class UsapTableEntry {
* 3) It fails and the new value isn't INVALID_ENTRY_VALUE, in which
* case the entry has already been cleared and re-used.
*
* In all three cases the goal of the caller has been met and we can
* return true.
* In all three cases the goal of the caller has been met, but only in
* the first case do we need to decrement the pool count.
*/
if (mStorage.compare_exchange_strong(storage, INVALID_ENTRY_VALUE)) {
close(storage.read_pipe_fd);
return true;
} else {
return false;
}
return true;
} else {
return false;
}
@@ -331,11 +333,24 @@ static void SigChldHandler(int /*signal_number*/) {
if (WIFEXITED(status)) {
async_safe_format_log(ANDROID_LOG_INFO, LOG_TAG,
"Process %d exited cleanly (%d)", pid, WEXITSTATUS(status));
// Check to see if the PID is in the USAP pool and remove it if it is.
if (RemoveUsapTableEntry(pid)) {
++usaps_removed;
}
} else if (WIFSIGNALED(status)) {
async_safe_format_log(ANDROID_LOG_INFO, LOG_TAG,
"Process %d exited due to signal %d (%s)%s", pid,
WTERMSIG(status), strsignal(WTERMSIG(status)),
WCOREDUMP(status) ? "; core dumped" : "");
// If the process exited due to a signal other than SIGTERM, check to see
// if the PID is in the USAP pool and remove it if it is. If the process
// was closed by the Zygote using SIGTERM then the USAP pool entry will
// have already been removed (see nativeEmptyUsapPool()).
if (WTERMSIG(status) != SIGTERM && RemoveUsapTableEntry(pid)) {
++usaps_removed;
}
}
// If the just-crashed process is the system_server, bring down zygote
@@ -346,11 +361,6 @@ static void SigChldHandler(int /*signal_number*/) {
"Exit zygote because system server (pid %d) has terminated", pid);
kill(getpid(), SIGKILL);
}
// Check to see if the PID is in the USAP pool and remove it if it is.
if (RemoveUsapTableEntry(pid)) {
++usaps_removed;
}
}
// Note that we shouldn't consider ECHILD an error because
@@ -1648,7 +1658,13 @@ static void com_android_internal_os_Zygote_nativeEmptyUsapPool(JNIEnv* env, jcla
auto entry_storage = entry.GetValues();
if (entry_storage.has_value()) {
kill(entry_storage.value().pid, SIGKILL);
kill(entry_storage.value().pid, SIGTERM);
// Clean up the USAP table entry here. This avoids a potential race
// where a newly created USAP might not be able to find a valid table
// entry if signal handler (which would normally do the cleanup) doesn't
// run between now and when the new process is created.
close(entry_storage.value().read_pipe_fd);
// Avoid a second atomic load by invalidating instead of clearing.
@@ -1678,6 +1694,16 @@ static jboolean com_android_internal_os_Zygote_nativeDisableExecuteOnly(JNIEnv*
return dl_iterate_phdr(disable_execute_only, nullptr) == 0;
}
static void com_android_internal_os_Zygote_nativeBlockSigTerm(JNIEnv* env, jclass) {
auto fail_fn = std::bind(ZygoteFailure, env, "usap", nullptr, _1);
BlockSignal(SIGTERM, fail_fn);
}
static void com_android_internal_os_Zygote_nativeUnblockSigTerm(JNIEnv* env, jclass) {
auto fail_fn = std::bind(ZygoteFailure, env, "usap", nullptr, _1);
UnblockSignal(SIGTERM, fail_fn);
}
static const JNINativeMethod gMethods[] = {
{ "nativeForkAndSpecialize",
"(II[II[[IILjava/lang/String;Ljava/lang/String;[I[IZLjava/lang/String;Ljava/lang/String;)I",
@@ -1708,7 +1734,11 @@ static const JNINativeMethod gMethods[] = {
{ "nativeEmptyUsapPool", "()V",
(void *) com_android_internal_os_Zygote_nativeEmptyUsapPool },
{ "nativeDisableExecuteOnly", "()Z",
(void *) com_android_internal_os_Zygote_nativeDisableExecuteOnly }
(void *) com_android_internal_os_Zygote_nativeDisableExecuteOnly },
{ "nativeBlockSigTerm", "()V",
(void* ) com_android_internal_os_Zygote_nativeBlockSigTerm },
{ "nativeUnblockSigTerm", "()V",
(void* ) com_android_internal_os_Zygote_nativeUnblockSigTerm }
};
int register_com_android_internal_os_Zygote(JNIEnv* env) {