From d7b3a369594ddaae56ece2a6b603c15a58f9b398 Mon Sep 17 00:00:00 2001 From: Martijn Coenen Date: Wed, 30 Jan 2019 11:17:52 +0100 Subject: [PATCH] Widen allowed UID range for webview zygote. The webview zygote is shared for all users on the system, and so unlike the app zygote, it can't use a single whitelisted UID range. For now, clamp to the upper bound of the UID range, until we have a better idea. This is still an improvement from the previous status quo, because it will prevent setuid/setgid into system users. Bug: 123597434 Test: builds, webview_zygote running Change-Id: Ia975826ed5b1f20cabb46f60f5951723b1ba80c9 --- core/java/android/webkit/WebViewZygote.java | 2 +- core/java/com/android/internal/os/ChildZygoteInit.java | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/core/java/android/webkit/WebViewZygote.java b/core/java/android/webkit/WebViewZygote.java index de1f3df614626..09aa066549cb9 100644 --- a/core/java/android/webkit/WebViewZygote.java +++ b/core/java/android/webkit/WebViewZygote.java @@ -163,7 +163,7 @@ public class WebViewZygote { TextUtils.join(",", Build.SUPPORTED_ABIS), null, // instructionSet Process.FIRST_ISOLATED_UID, - Process.LAST_ISOLATED_UID); + Integer.MAX_VALUE); // TODO(b/123615476) deal with user-id ranges properly ZygoteProcess.waitForConnectionToZygote(sZygote.getPrimarySocketAddress()); if (sPackageOriginalAppInfo.sourceDir.equals(sPackage.applicationInfo.sourceDir)) { diff --git a/core/java/com/android/internal/os/ChildZygoteInit.java b/core/java/com/android/internal/os/ChildZygoteInit.java index a052a3b3ab6ac..cc74863632dd9 100644 --- a/core/java/com/android/internal/os/ChildZygoteInit.java +++ b/core/java/com/android/internal/os/ChildZygoteInit.java @@ -98,9 +98,11 @@ public class ChildZygoteInit { throw new RuntimeException("Passed in UID range is invalid, min > max."); } - // Verify the UIDs are in the isolated UID range, as that's the only thing that we should - // be forking right now - if (!Process.isIsolated(uidGidMin) || !Process.isIsolated(uidGidMax)) { + // Verify the UIDs at least do not include system UIDs; we can't easily verify there + // are just isolated UIDs in the range, because for the webview zygote, there is no + // single range that captures all possible isolated UIDs. + // TODO(b/123615476) narrow this down + if (uidGidMin < Process.FIRST_ISOLATED_UID) { throw new RuntimeException("Passed in UID range does not map to isolated processes."); }