From ba160a28fda89c7db9c35db038dbe5e2654da473 Mon Sep 17 00:00:00 2001
From: Mitch Phillips <mitchp@google.com>
Date: Mon, 13 Apr 2020 11:37:49 -0700
Subject: [PATCH] Check 64-to-32-bit transition in system_server for Tagged
 Ptrs [#2].

Note: Same patch as previously reverted with the fix.

The check whether the system (kernel) supports Tagged Pointers is done
in the 64-bit system server. This can spawn 32-bit apps, and we
think that the app supports tagged pointers.

There's no good way to check the compatibility in the child before we
spawn it, so we do the next best thing. If we know the child won't
support it (i.e. the child is 32-bit), we don't ask for Tagged Ptrs.
Otherwise, if the parent supports tagged pointers, and there's no
bitness transition, the child should also support.

Bug: 153114333
Test: Build flame, launch 64-bit apps that spawns 32-bit subprocesses,
verify that you get no 're-enabling tagging' messages.
Test: atest -a CtsTaggingHostTestCases

Change-Id: Ida6aa56093de6b6b03e78e6295bea546219ad864
Merged-In: Ida6aa56093de6b6b03e78e6295bea546219ad864
---
 .../com/android/server/am/ProcessList.java    | 50 +++++++++++++++----
 1 file changed, 41 insertions(+), 9 deletions(-)

diff --git a/services/core/java/com/android/server/am/ProcessList.java b/services/core/java/com/android/server/am/ProcessList.java
index 89fa02bbbd64b..cce749d5a7efc 100644
--- a/services/core/java/com/android/server/am/ProcessList.java
+++ b/services/core/java/com/android/server/am/ProcessList.java
@@ -1668,6 +1668,33 @@ public final class ProcessList {
         return gidArray;
     }
 
+    private boolean shouldEnableTaggedPointers(ProcessRecord app) {
+        // Ensure we have platform + kernel support for TBI.
+        if (!Zygote.nativeSupportsTaggedPointers()) {
+            return false;
+        }
+
+        // Check to ensure the app hasn't explicitly opted-out of TBI via. the manifest attribute.
+        if (!app.info.allowsNativeHeapPointerTagging()) {
+            return false;
+        }
+
+        // Check to see that the compat feature for TBI is enabled.
+        if (!mPlatformCompat.isChangeEnabled(NATIVE_HEAP_POINTER_TAGGING, app.info)) {
+            return false;
+        }
+
+        return true;
+    }
+
+    private int decideTaggingLevel(ProcessRecord app) {
+        if (shouldEnableTaggedPointers(app)) {
+            return Zygote.MEMORY_TAG_LEVEL_TBI;
+        }
+
+        return 0;
+    }
+
     private int decideGwpAsanLevel(ProcessRecord app) {
         // Look at the process attribute first.
        if (app.processInfo != null
@@ -1856,15 +1883,6 @@ public final class ProcessList {
                 runtimeFlags |= Zygote.USE_APP_IMAGE_STARTUP_CACHE;
             }
 
-            if (Zygote.nativeSupportsTaggedPointers()) {
-                // Enable heap pointer tagging if supported by the kernel, unless disabled by the
-                // app manifest, target sdk level, or compat feature.
-                if (app.info.allowsNativeHeapPointerTagging()
-                        && mPlatformCompat.isChangeEnabled(NATIVE_HEAP_POINTER_TAGGING, app.info)) {
-                    runtimeFlags |= Zygote.MEMORY_TAG_LEVEL_TBI;
-                }
-            }
-
             runtimeFlags |= decideGwpAsanLevel(app);
 
             String invokeWith = null;
@@ -1895,6 +1913,20 @@ public final class ProcessList {
             app.setRequiredAbi(requiredAbi);
             app.instructionSet = instructionSet;
 
+            // If instructionSet is non-null, this indicates that the system_server is spawning a
+            // process with an ISA that may be different from its own. System (kernel and hardware)
+            // compatililty for these features is checked in the decideTaggingLevel in the
+            // system_server process (not the child process). As TBI is only supported in aarch64,
+            // we can simply ensure that the new process is also aarch64. This prevents the mismatch
+            // where a 64-bit system server spawns a 32-bit child that thinks it should enable some
+            // tagging variant. Theoretically, a 32-bit system server could exist that spawns 64-bit
+            // processes, in which case the new process won't get any tagging. This is fine as we
+            // haven't seen this configuration in practice, and we can reasonable assume that if
+            // tagging is desired, the system server will be 64-bit.
+            if (instructionSet == null || instructionSet.equals("arm64")) {
+                runtimeFlags |= decideTaggingLevel(app);
+            }
+
             // the per-user SELinux context must be set
             if (TextUtils.isEmpty(app.info.seInfoUser)) {
                 Slog.wtf(ActivityManagerService.TAG, "SELinux tag not defined",