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

Commit cbf47916 authored by Jeff Sharkey's avatar Jeff Sharkey
Browse files

Fix lock inversion in PackageInstaller. 

In a small handful of cases individual sessions call up into the
installer while holding their local locks.  Defend against this by
treating most InternalCallback events as async.  For sealed events,
perform the upcall outside of the session lock.

Bug: 17482676
Change-Id: I265d981c98c8928a0fced09d8b029ca16eb650d9
parent e980804d

services/core/java/com/android/server/pm/PackageInstallerService.java

+19 −8
Original line number Diff line number Diff line
@@ -144,6 +144,7 @@ public class PackageInstallerService extends IPackageInstaller.Stub { 


    private final File mStagingDir;

    private final HandlerThread mInstallThread;

    private final Handler mInstallHandler;



    private final Callbacks mCallbacks;
@@ -188,6 +189,8 @@ public class PackageInstallerService extends IPackageInstaller.Stub { 
        mInstallThread = new HandlerThread(TAG);

        mInstallThread.start();



        mInstallHandler = new Handler(mInstallThread.getLooper());



        mCallbacks = new Callbacks(mInstallThread.getLooper());



        mSessionsFile = new AtomicFile(
@@ -961,13 +964,19 @@ public class PackageInstallerService extends IPackageInstaller.Stub { 
            mCallbacks.notifySessionProgressChanged(session.sessionId, session.userId, progress);

        }



        public void onSessionFinished(PackageInstallerSession session, boolean success) {

        public void onSessionFinished(final PackageInstallerSession session, boolean success) {

            mCallbacks.notifySessionFinished(session.sessionId, session.userId, success);



            mInstallHandler.post(new Runnable() {

                @Override

                public void run() {

                    synchronized (mSessions) {

                        mSessions.remove(session.sessionId);

                        mHistoricalSessions.put(session.sessionId, session);

                        writeSessionsLocked();

                    }

            writeSessionsAsync();

                }

            });

        }



        public void onSessionPrepared(PackageInstallerSession session) {
@@ -976,11 +985,13 @@ public class PackageInstallerService extends IPackageInstaller.Stub { 
            writeSessionsAsync();

        }



        public void onSessionSealed(PackageInstallerSession session) {

        public void onSessionSealedBlocking(PackageInstallerSession session) {

            // It's very important that we block until we've recorded the

            // session as being sealed, since we never want to allow mutation

            // after sealing.

            synchronized (mSessions) {

                writeSessionsLocked();

            }

        }

    }

}

services/core/java/com/android/server/pm/PackageInstallerSession.java

+12 −7
Original line number Diff line number Diff line
@@ -410,7 +410,9 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
    public void commit(IntentSender statusReceiver) {

        Preconditions.checkNotNull(statusReceiver);



        final boolean wasSealed;

        synchronized (mLock) {

            wasSealed = mSealed;

            if (!mSealed) {

                // Verify that all writers are hands-off

                for (FileBridge bridge : mBridges) {
@@ -418,17 +420,20 @@ public class PackageInstallerSession extends IPackageInstallerSession.Stub { 
                        throw new SecurityException("Files still open");

                    }

                }



                // Persist the fact that we've sealed ourselves to prevent

                // mutations of any hard links we create.

                mSealed = true;

                mCallback.onSessionSealed(this);

            }

            }



            // Client staging is fully done at this point

            mClientProgress = 1f;

            computeProgressLocked(true);

        }



        if (!wasSealed) {

            // Persist the fact that we've sealed ourselves to prevent

            // mutations of any hard links we create. We do this without holding

            // the session lock, since otherwise it's a lock inversion.

            mCallback.onSessionSealedBlocking(this);

        }



        // This ongoing commit should keep session active, even though client

        // will probably close their end.