Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit de2906af authored by Peter Collingbourne's avatar Peter Collingbourne
Browse files

Reset PAC keys on thread creation instead of on zygote fork. 

Resetting PAC keys on fork appears to lead to a number of problems. One
problem is that we are constrained in where we can run C++ code after
forking, and with ART those places are implementation-defined. For
example, in app zygotes, ART turns out to insert "interpreter frames"
in the stack trace. Returning into these interpreter frames may lead
to crashes due to failing the ROP protection check on return.

It seems better to reset keys on thread creation instead. We only need
to reset IA because only this key needs to be reset for reverse-edge
PAC, and resetting the other keys may be incompatible with future ABIs.

Chrome (and potentially other applications) has a sandbox that prevents
the use of the prctl, so we restrict its use to applications targeting
S and above.

Bug: 183024045
Change-Id: I1e6502a7d7df319d424e2b0f653aad9a343ae71b
parent 59437812

core/jni/com_android_internal_os_Zygote.cpp

+0 −21
Original line number Diff line number Diff line
@@ -1015,21 +1015,6 @@ static void ClearUsapTable() { 
  gUsapPoolCount = 0;

}



NO_PAC_FUNC

static void PAuthKeyChange(JNIEnv* env) {

#ifdef __aarch64__

  unsigned long int hwcaps = getauxval(AT_HWCAP);

  if (hwcaps & HWCAP_PACA) {

    const unsigned long key_mask = PR_PAC_APIAKEY | PR_PAC_APIBKEY |

                                   PR_PAC_APDAKEY | PR_PAC_APDBKEY | PR_PAC_APGAKEY;

    if (prctl(PR_PAC_RESET_KEYS, key_mask, 0, 0, 0) != 0) {

      ALOGE("Failed to change the PAC keys: %s", strerror(errno));

      RuntimeAbort(env, __LINE__, "PAC key change failed.");

    }

  }

#endif

}



// Create an app data directory over tmpfs overlayed CE / DE storage, and bind mount it

// from the actual app data directory in data mirror.

static bool createAndMountAppData(std::string_view package_name,
@@ -1980,7 +1965,6 @@ void zygote::ZygoteFailure(JNIEnv* env, 
}



// Utility routine to fork a process from the zygote.

NO_PAC_FUNC

pid_t zygote::ForkCommon(JNIEnv* env, bool is_system_server,

                         const std::vector<int>& fds_to_close,

                         const std::vector<int>& fds_to_ignore,
@@ -2035,7 +2019,6 @@ pid_t zygote::ForkCommon(JNIEnv* env, bool is_system_server, 
    }



    // The child process.

    PAuthKeyChange(env);

    PreApplicationInit();



    // Clean up any descriptors which must be closed immediately
@@ -2067,7 +2050,6 @@ static void com_android_internal_os_Zygote_nativePreApplicationInit(JNIEnv*, jcl 
  PreApplicationInit();

}



NO_PAC_FUNC

static jint com_android_internal_os_Zygote_nativeForkAndSpecialize(

        JNIEnv* env, jclass, jint uid, jint gid, jintArray gids, jint runtime_flags,

        jobjectArray rlimits, jint mount_external, jstring se_info, jstring nice_name,
@@ -2117,7 +2099,6 @@ static jint com_android_internal_os_Zygote_nativeForkAndSpecialize( 
    return pid;

}



NO_PAC_FUNC

static jint com_android_internal_os_Zygote_nativeForkSystemServer(

        JNIEnv* env, jclass, uid_t uid, gid_t gid, jintArray gids,

        jint runtime_flags, jobjectArray rlimits, jlong permitted_capabilities,
@@ -2189,7 +2170,6 @@ static jint com_android_internal_os_Zygote_nativeForkSystemServer( 
 * @param is_priority_fork  Controls the nice level assigned to the newly created process

 * @return child pid in the parent, 0 in the child

 */

NO_PAC_FUNC

static jint com_android_internal_os_Zygote_nativeForkApp(JNIEnv* env,

                                                         jclass,

                                                         jint read_pipe_fd,
@@ -2204,7 +2184,6 @@ static jint com_android_internal_os_Zygote_nativeForkApp(JNIEnv* env, 
                            args_known == JNI_TRUE, is_priority_fork == JNI_TRUE, true);

}



NO_PAC_FUNC

int zygote::forkApp(JNIEnv* env,

                    int read_pipe_fd,

                    int write_pipe_fd,

core/jni/com_android_internal_os_Zygote.h

+0 −14
Original line number Diff line number Diff line
@@ -20,18 +20,6 @@ 
#define LOG_TAG "Zygote"

#define ATRACE_TAG ATRACE_TAG_DALVIK



/* Functions in the callchain during the fork shall not be protected with

   Armv8.3-A Pointer Authentication, otherwise child will not be able to return. */

#ifdef __ARM_FEATURE_PAC_DEFAULT

#ifdef __ARM_FEATURE_BTI_DEFAULT

#define NO_PAC_FUNC __attribute__((target("branch-protection=bti")))

#else

#define NO_PAC_FUNC __attribute__((target("branch-protection=none")))

#endif /* __ARM_FEATURE_BTI_DEFAULT */

#else /* !__ARM_FEATURE_PAC_DEFAULT */

#define NO_PAC_FUNC

#endif /* __ARM_FEATURE_PAC_DEFAULT */



#include <jni.h>

#include <vector>

#include <android-base/stringprintf.h>
@@ -42,7 +30,6 @@ 
namespace android {

namespace zygote {



NO_PAC_FUNC

pid_t ForkCommon(JNIEnv* env,bool is_system_server,

                 const std::vector<int>& fds_to_close,

                 const std::vector<int>& fds_to_ignore,
@@ -57,7 +44,6 @@ pid_t ForkCommon(JNIEnv* env,bool is_system_server, 
 * communication is required. Is_priority_fork should be true if this is on the app startup

 * critical path. Purge specifies that unused pages should be purged before the fork.

 */

NO_PAC_FUNC

int forkApp(JNIEnv* env,

            int read_pipe_fd,

            int write_pipe_fd,

core/jni/com_android_internal_os_ZygoteCommandBuffer.cpp

+0 −1
Original line number Diff line number Diff line
@@ -365,7 +365,6 @@ void com_android_internal_os_ZygoteCommandBuffer_nativeReadFullyAndReset(JNIEnv* 
// We only process fork commands if the peer uid matches expected_uid.

// For every fork command after the first, we check that the requested uid is at

// least minUid.

NO_PAC_FUNC

jboolean com_android_internal_os_ZygoteCommandBuffer_nativeForkRepeatedly(

            JNIEnv* env,

            jclass,