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

Commit 35a397c4 authored by Tomislav Novak's avatar Tomislav Novak
Browse files

StrictMode: fix race condition in onVmPolicyViolation 

There's a TOCTOU race condition in onVmPolicyViolation() that can cause
a NullPointerException if multiple threads trigger a violation and a
penalty listener is set. For example:

1. Thread 1 passes the mCallbackExecutor null check and calls execute()
2. T2 passes the same check and then gets preempted
3. Runnable queued by T1 temporarily replaces sVmPolicy with LAX (which
   has a null executor) by calling allowVmViolations()
4. T2 calls execute() on sVmPolicy.mCallbackExecutor, which is now null

Fix it by using the same VmPolicy object throughout onVmPolicyViolation.

Test: atest StrictModeTest
Change-Id: Ifa20253ea936b8d3d8c3719c3278bfaccbdf8275
parent 6b30dd66

core/java/android/os/StrictMode.java

+8 −7
Original line number Diff line number Diff line
@@ -2436,11 +2436,12 @@ public final class StrictMode { 


    /** @hide */

    public static void onVmPolicyViolation(Violation violation, boolean forceDeath) {

        final boolean penaltyDropbox = (sVmPolicy.mask & PENALTY_DROPBOX) != 0;

        final boolean penaltyDeath = ((sVmPolicy.mask & PENALTY_DEATH) != 0) || forceDeath;

        final boolean penaltyLog = (sVmPolicy.mask & PENALTY_LOG) != 0;

        final VmPolicy vmPolicy = getVmPolicy();

        final boolean penaltyDropbox = (vmPolicy.mask & PENALTY_DROPBOX) != 0;

        final boolean penaltyDeath = ((vmPolicy.mask & PENALTY_DEATH) != 0) || forceDeath;

        final boolean penaltyLog = (vmPolicy.mask & PENALTY_LOG) != 0;



        final int penaltyMask = (sVmPolicy.mask & PENALTY_ALL);

        final int penaltyMask = (vmPolicy.mask & PENALTY_ALL);

        final ViolationInfo info = new ViolationInfo(violation, penaltyMask);



        // Erase stuff not relevant for process-wide violations
@@ -2493,10 +2494,10 @@ public final class StrictMode { 


        // If penaltyDeath, we can't guarantee this callback finishes before the process dies for

        // all executors. penaltyDeath supersedes penaltyCallback.

        if (sVmPolicy.mListener != null && sVmPolicy.mCallbackExecutor != null) {

            final OnVmViolationListener listener = sVmPolicy.mListener;

        if (vmPolicy.mListener != null && vmPolicy.mCallbackExecutor != null) {

            final OnVmViolationListener listener = vmPolicy.mListener;

            try {

                sVmPolicy.mCallbackExecutor.execute(

                vmPolicy.mCallbackExecutor.execute(

                        () -> {

                            // Lift violated policy to prevent infinite recursion.

                            VmPolicy oldPolicy = allowVmViolations();