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

Commit 5df3f27d authored by Riddle Hsu's avatar Riddle Hsu
Browse files

Drop task snapshot with invalid hw buffer 

The notifyTaskSnapshotChanged may not be always called after
Binder.clearCallingIdentity. And because there was commit
67b09024 that avoids keeping unnecessary buffer in remote
processes, it may mis-close the buffer in system server if
the calling pid isn't cleared on binder thread.

e.g. there are several suspects from ActivityTaskManagerInternal
Though it might be the responsibility of the caller to clear
binder identity before calling internal interfaces.

Instead of adding clearCallingIdentity every where:
1. Explicitly set the listener as local to skip closing the buffer
   so it is independent of which thread is calling.
2. Protect the persister when using the buffer.

Bug: 192143094
Bug: 220659717
Bug: 223950159
Bug: 225794452
Test: atest TaskSnapshotPersisterLoaderTest
Change-Id: I94e8e706e56a73fffe8189023bcef5efd797a24c
parent 2ea87d4d

core/java/android/app/TaskStackListener.java

+9 −3
Original line number Diff line number Diff line
@@ -19,7 +19,6 @@ package android.app; 
import android.app.ActivityManager.RunningTaskInfo;

import android.compat.annotation.UnsupportedAppUsage;

import android.content.ComponentName;

import android.os.Binder;

import android.os.Build;

import android.os.RemoteException;

import android.window.TaskSnapshot;
@@ -32,10 +31,18 @@ import android.window.TaskSnapshot; 
 */

public abstract class TaskStackListener extends ITaskStackListener.Stub {



    /** Whether this listener and the callback dispatcher are in different processes. */

    private boolean mIsRemote = true;



    @UnsupportedAppUsage

    public TaskStackListener() {

    }



    /** Indicates that this listener lives in system server. */

    public void setIsLocal() {

        mIsRemote = false;

    }



    @Override

    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    public void onTaskStackChanged() throws RemoteException {
@@ -154,8 +161,7 @@ public abstract class TaskStackListener extends ITaskStackListener.Stub { 
    @Override

    @UnsupportedAppUsage(maxTargetSdk = Build.VERSION_CODES.R, trackingBug = 170729553)

    public void onTaskSnapshotChanged(int taskId, TaskSnapshot snapshot) throws RemoteException {

        if (Binder.getCallingPid() != android.os.Process.myPid()

                && snapshot != null && snapshot.getHardwareBuffer() != null) {

        if (mIsRemote && snapshot != null && snapshot.getHardwareBuffer() != null) {

            // Preemptively clear any reference to the buffer

            snapshot.getHardwareBuffer().close();

        }

services/core/java/com/android/server/wm/TaskChangeNotificationController.java

+4 −0
Original line number Diff line number Diff line
@@ -20,6 +20,7 @@ import android.app.ActivityManager; 
import android.app.ActivityManager.RunningTaskInfo;

import android.app.ITaskStackListener;

import android.app.TaskInfo;

import android.app.TaskStackListener;

import android.content.ComponentName;

import android.os.Binder;

import android.os.Handler;
@@ -286,6 +287,9 @@ class TaskChangeNotificationController { 
        if (listener instanceof Binder) {

            synchronized (mLocalTaskStackListeners) {

                if (!mLocalTaskStackListeners.contains(listener)) {

                    if (listener instanceof TaskStackListener) {

                        ((TaskStackListener) listener).setIsLocal();

                    }

                    mLocalTaskStackListeners.add(listener);

                }

            }

services/core/java/com/android/server/wm/TaskSnapshotController.java

+6 −1
Original line number Diff line number Diff line
@@ -480,12 +480,17 @@ class TaskSnapshotController { 
        }

        final HardwareBuffer buffer = screenshotBuffer == null ? null

                : screenshotBuffer.getHardwareBuffer();

        if (buffer == null || buffer.getWidth() <= 1 || buffer.getHeight() <= 1) {

        if (isInvalidHardwareBuffer(buffer)) {

            return null;

        }

        return screenshotBuffer;

    }



    static boolean isInvalidHardwareBuffer(HardwareBuffer buffer) {

        return buffer == null || buffer.isClosed() // This must be checked before getting size.

                || buffer.getWidth() <= 1 || buffer.getHeight() <= 1;

    }



    @Nullable

    TaskSnapshot snapshotTask(Task task) {

        return snapshotTask(task, PixelFormat.UNKNOWN);

services/core/java/com/android/server/wm/TaskSnapshotPersister.java

+4 −0
Original line number Diff line number Diff line
@@ -407,6 +407,10 @@ class TaskSnapshotPersister { 
        }



        boolean writeBuffer() {

            if (TaskSnapshotController.isInvalidHardwareBuffer(mSnapshot.getHardwareBuffer())) {

                Slog.e(TAG, "Invalid task snapshot hw buffer, taskId=" + mTaskId);

                return false;

            }

            final Bitmap bitmap = Bitmap.wrapHardwareBuffer(

                    mSnapshot.getHardwareBuffer(), mSnapshot.getColorSpace());

            if (bitmap == null) {

services/core/java/com/android/server/wm/WindowManagerService.java

+11 −6
Original line number Diff line number Diff line
@@ -9045,6 +9045,8 @@ public class WindowManagerService extends IWindowManager.Stub 
        }



        TaskSnapshot taskSnapshot;

        final long token = Binder.clearCallingIdentity();

        try {

            synchronized (mGlobalLock) {

                Task task = mRoot.anyTaskForId(taskId, MATCH_ATTACHED_TASK_OR_RECENT_TASKS);

                if (task == null) {
@@ -9053,6 +9055,9 @@ public class WindowManagerService extends IWindowManager.Stub 
                }

                taskSnapshot = mTaskSnapshotController.captureTaskSnapshot(task, false);

            }

        } finally {

            Binder.restoreCallingIdentity(token);

        }



        if (taskSnapshot == null || taskSnapshot.getHardwareBuffer() == null) {

            return null;