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

Commit ae93714d authored by Chris Wailes's avatar Chris Wailes
Browse files

Improved file descriptor cleanup in Zygote. 

Enabling the blastula pool exercised a previously unused codepath that
wrote to the log.  This caused a socket to be opened when it wasn't
expected to be.  This patch re-closes the logging socket before applying
the SELinux policy for the new application.

This patch also adds code to clean up blastula table entries in
processes spawned from the Zygote and disables the blastula pool code
for child zygotes.

Bug: 123409530
Test: make & flash & boot & check logs for SELinux denials
Change-Id: I560cdf9c42502574d25ab25a0f8afa3eb6de307f
parent 870edccb

core/java/android/os/ZygoteProcess.java

+1 −1
Original line number Diff line number Diff line
@@ -444,7 +444,7 @@ public class ZygoteProcess { 
                // If there was an IOException using the blastula pool we will log the error and

                // attempt to start the process through the Zygote.

                Log.e(LOG_TAG, "IO Exception while communicating with blastula pool - "

                               + ex.toString());

                               + ex.getMessage());

            } finally {

                try {

                    blastulaSessionSocket.close();

core/java/com/android/internal/os/Zygote.java

+16 −11
Original line number Diff line number Diff line
@@ -427,6 +427,12 @@ public final class Zygote { 
                defaultValue);

    }



    protected static void emptyBlastulaPool() {

        nativeEmptyBlastulaPool();

    }



    private static native void nativeEmptyBlastulaPool();



    /**

     * Returns the value of a system property converted to a boolean using specific logic.

     *
@@ -520,7 +526,7 @@ public final class Zygote { 
        LocalSocket sessionSocket = null;

        DataOutputStream blastulaOutputStream = null;

        Credentials peerCredentials = null;

        String[] argStrings = null;

        ZygoteArguments args = null;



        while (true) {

            try {
@@ -533,25 +539,24 @@ public final class Zygote { 


                peerCredentials = sessionSocket.getPeerCredentials();



                argStrings = readArgumentList(blastulaReader);

                String[] argStrings = readArgumentList(blastulaReader);



                if (argStrings != null) {

                    args = new ZygoteArguments(argStrings);



                    // TODO (chriswailes): Should this only be run for debug builds?

                    validateBlastulaCommand(args);

                    break;

                } else {

                    Log.e("Blastula", "Truncated command received.");

                    IoUtils.closeQuietly(sessionSocket);

                }

            } catch (IOException ioEx) {

                Log.e("Blastula", "Failed to read command: " + ioEx.getMessage());

            } catch (Exception ex) {

                Log.e("Blastula", ex.getMessage());

                IoUtils.closeQuietly(sessionSocket);

            }

        }



        ZygoteArguments args = new ZygoteArguments(argStrings);



        // TODO (chriswailes): Should this only be run for debug builds?

        validateBlastulaCommand(args);



        applyUidSecurityPolicy(args, peerCredentials);

        applyDebuggerSystemProperty(args);

core/java/com/android/internal/os/ZygoteServer.java

+84 −44
Original line number Diff line number Diff line
@@ -67,6 +67,15 @@ class ZygoteServer { 
    /** The default value used for the BLASTULA_POOL_SIZE_MIN device property */

    private static final String BLASTULA_POOL_SIZE_MIN_DEFAULT = "1";



    /**

     * Indicates if this Zygote server can support a blastula pool.  Currently this should only be

     * true for the primary and secondary Zygotes, and not the App Zygotes or the WebView Zygote.

     *

     * TODO (chriswailes): Make this an explicit argument to the constructor

     */



    private final boolean mBlastulaPoolSupported;



    /**

     * If the blastula pool should be created and used to start applications.

     *
@@ -127,6 +136,8 @@ class ZygoteServer { 
        mBlastulaPoolEventFD = null;

        mZygoteSocket = null;

        mBlastulaPoolSocket = null;



        mBlastulaPoolSupported = false;

    }



    /**
@@ -151,12 +162,18 @@ class ZygoteServer { 
        }



        fetchBlastulaPoolPolicyProps();



        mBlastulaPoolSupported = true;

    }



    void setForkChild() {

        mIsForkChild = true;

    }



    public boolean isBlastulaPoolEnabled() {

        return mBlastulaPoolEnabled;

    }



    /**

     * Registers a server socket for zygote command connections. This opens the server socket

     * at the specified name in the abstract socket namespace.
@@ -282,8 +299,8 @@ class ZygoteServer { 
     *         this function will return a Runnable object representing the new application that is

     *         passed up from blastulaMain.

     */

    private Runnable fillBlastulaPool(int[] sessionSocketRawFDs) {

        if (mBlastulaPoolEnabled) {



    Runnable fillBlastulaPool(int[] sessionSocketRawFDs) {

        Trace.traceBegin(Trace.TRACE_TAG_ACTIVITY_MANAGER, "Zygote:FillBlastulaPool");



        int blastulaPoolCount = Zygote.getBlastulaPoolCount();
@@ -314,7 +331,6 @@ class ZygoteServer { 
        }



        Trace.traceEnd(Trace.TRACE_TAG_ACTIVITY_MANAGER);

        }



        return null;

    }
@@ -334,12 +350,26 @@ class ZygoteServer { 
        while (true) {

            fetchBlastulaPoolPolicyPropsWithMinInterval();



            int[] blastulaPipeFDs = Zygote.getBlastulaPipeFDs();

            int[] blastulaPipeFDs = null;

            StructPollfd[] pollFDs = null;



            // Allocate enough space for the poll structs, taking into account

            // the state of the blastula pool for this Zygote (could be a

            // regular Zygote, a WebView Zygote, or an AppZygote).

            if (mBlastulaPoolEnabled) {

                blastulaPipeFDs = Zygote.getBlastulaPipeFDs();

                pollFDs = new StructPollfd[socketFDs.size() + 1 + blastulaPipeFDs.length];

            } else {

                pollFDs = new StructPollfd[socketFDs.size()];

            }



            // Space for all of the socket FDs, the Blastula Pool Event FD, and

            // all of the open blastula read pipe FDs.

            StructPollfd[] pollFDs =

                new StructPollfd[socketFDs.size() + 1 + blastulaPipeFDs.length];

            /*

             * For reasons of correctness the blastula pool pipe and event FDs

             * must be processed before the session and server sockets.  This

             * is to ensure that the blastula pool accounting information is

             * accurate when handling other requests like API blacklist

             * exemptions.

             */



            int pollIndex = 0;

            for (FileDescriptor socketFD : socketFDs) {
@@ -350,6 +380,8 @@ class ZygoteServer { 
            }



            final int blastulaPoolEventFDIndex = pollIndex;



            if (mBlastulaPoolEnabled) {

                pollFDs[pollIndex] = new StructPollfd();

                pollFDs[pollIndex].fd = mBlastulaPoolEventFD;

                pollFDs[pollIndex].events = (short) POLLIN;
@@ -364,6 +396,7 @@ class ZygoteServer { 
                    pollFDs[pollIndex].events = (short) POLLIN;

                    ++pollIndex;

                }

            }



            try {

                Os.poll(pollFDs, -1);
@@ -371,6 +404,8 @@ class ZygoteServer { 
                throw new RuntimeException("poll failed", ex);

            }



            boolean blastulaPoolFDRead = false;



            while (--pollIndex >= 0) {

                if ((pollFDs[pollIndex].revents & POLLIN) == 0) {

                    continue;
@@ -480,6 +515,12 @@ class ZygoteServer { 
                        Zygote.removeBlastulaTableEntry((int) messagePayload);

                    }



                    blastulaPoolFDRead = true;

                }

            }



            // Check to see if the blastula pool needs to be refilled.

            if (blastulaPoolFDRead) {

                int[] sessionSocketRawFDs =

                        socketFDs.subList(1, socketFDs.size())

                                .stream()
@@ -495,4 +536,3 @@ class ZygoteServer { 
        }

    }

}
 
}

core/jni/com_android_internal_os_Zygote.cpp

+36 −1
Original line number Diff line number Diff line
@@ -216,6 +216,10 @@ class BlastulaTableEntry { 
    }

  }



  void Clear() {

    mStorage.store(INVALID_ENTRY_VALUE);

  }



  /**

   * @return A copy of the data stored in this entry.

   */
@@ -1159,6 +1163,14 @@ static void UnblockSignal(int signum, fail_fn_t fail_fn) { 
  }

}



static void ClearBlastulaTable() {

  for (BlastulaTableEntry& entry : gBlastulaTable) {

    entry.Clear();

  }



  gBlastulaPoolCount = 0;

}



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

static pid_t ForkCommon(JNIEnv* env, bool is_system_server,

                        const std::vector<int>& fds_to_close,
@@ -1201,6 +1213,9 @@ static pid_t ForkCommon(JNIEnv* env, bool is_system_server, 
    // Clean up any descriptors which must be closed immediately

    DetachDescriptors(env, fds_to_close, fail_fn);



    // Invalidate the entries in the blastula table.

    ClearBlastulaTable();



    // Re-open all remaining open file descriptors so that they aren't shared

    // with the zygote across a fork.

    gOpenFdTable->ReopenOrDetach(fail_fn);
@@ -1887,6 +1902,24 @@ static jint com_android_internal_os_Zygote_nativeGetBlastulaPoolCount(JNIEnv* en 
  return gBlastulaPoolCount;

}



/**

 * Kills all processes currently in the blastula pool.

 *

 * @param env  Managed runtime environment

 * @return The number of blastulas currently in the blastula pool

 */

static void com_android_internal_os_Zygote_nativeEmptyBlastulaPool(JNIEnv* env, jclass) {

  for (auto& entry : gBlastulaTable) {

    auto entry_storage = entry.GetValues();



    if (entry_storage.has_value()) {

      kill(entry_storage.value().pid, SIGKILL);

      entry.Clear();

      --gBlastulaPoolCount;

    }

  }

}



static const JNINativeMethod gMethods[] = {

    { "nativeSecurityInit", "()V",

      (void *) com_android_internal_os_Zygote_nativeSecurityInit },
@@ -1917,7 +1950,9 @@ static const JNINativeMethod gMethods[] = { 
    { "nativeGetBlastulaPoolEventFD", "()I",

      (void *) com_android_internal_os_Zygote_nativeGetBlastulaPoolEventFD },

    { "nativeGetBlastulaPoolCount", "()I",

      (void *) com_android_internal_os_Zygote_nativeGetBlastulaPoolCount }

      (void *) com_android_internal_os_Zygote_nativeGetBlastulaPoolCount },

    { "nativeEmptyBlastulaPool", "()V",

      (void *) com_android_internal_os_Zygote_nativeEmptyBlastulaPool }

};



int register_com_android_internal_os_Zygote(JNIEnv* env) {