From 2faa631db565e143f6dd10c0127388b44798e67c Mon Sep 17 00:00:00 2001
From: Garfield Tan <xutan@google.com>
Date: Tue, 23 Jun 2020 15:46:10 -0700
Subject: Read everything and return NULL in SurfaceControl.

We used to early return when we got an invalid SurfaceControl for any
reason, but that'll mess up the Parcel location and make all following
reading from this Parcel fail in a very obscure way.

To avoid that let's read up everything SurfaceControl wrote and return
NULL if it indeed is invalid.

Also change to call readStrongBinder() that can return status_t, and
check all status_t returned by readStrongBinder() and
readNullableStrongBinder() to catch all cases.

Note this issue came to ARC++ because our ArcSystemUI crashed earlier,
which is the root cause to the invalid SurfaceControl. Therefore this
change doesn't aim to fix any issue in normal code path.

Bug: 159627603
Test: atest ActivityVisibilityTests#testTurnScreenOnSingleTask failed
with using invalid SurfaceControl rather than ClassNotFoundException.

Change-Id: Icd2f694f2696458041f449f74186653791236010
---
 libs/gui/SurfaceControl.cpp | 27 +++++++++++++++++++--------
 1 file changed, 19 insertions(+), 8 deletions(-)

(limited to 'libs/gui/SurfaceControl.cpp')

diff --git a/libs/gui/SurfaceControl.cpp b/libs/gui/SurfaceControl.cpp
index a332a1f2a8..8dcb71bcfc 100644
--- a/libs/gui/SurfaceControl.cpp
+++ b/libs/gui/SurfaceControl.cpp
@@ -178,17 +178,28 @@ void SurfaceControl::writeToParcel(Parcel* parcel)
 }
 
 sp<SurfaceControl> SurfaceControl::readFromParcel(const Parcel* parcel) {
-    sp<IBinder> client = parcel->readStrongBinder();
-    sp<IBinder> handle = parcel->readStrongBinder();
-    if (client == nullptr || handle == nullptr)
-    {
-        ALOGE("Invalid parcel");
-        return nullptr;
+    bool invalidParcel = false;
+    status_t status;
+    sp<IBinder> client;
+    if ((status = parcel->readStrongBinder(&client)) != OK) {
+        ALOGE("Failed to read client: %s", statusToString(status).c_str());
+        invalidParcel = true;
+    }
+    sp<IBinder> handle;
+    if ((status = parcel->readStrongBinder(&handle)) != OK) {
+        ALOGE("Failed to read handle: %s", statusToString(status).c_str());
+        invalidParcel = true;
     }
     sp<IBinder> gbp;
-    parcel->readNullableStrongBinder(&gbp);
-
+    if ((status = parcel->readNullableStrongBinder(&gbp)) != OK) {
+        ALOGE("Failed to read gbp: %s", statusToString(status).c_str());
+        invalidParcel = true;
+    }
     uint32_t transformHint = parcel->readUint32();
+
+    if (invalidParcel) {
+        return nullptr;
+    }
     // We aren't the original owner of the surface.
     return new SurfaceControl(new SurfaceComposerClient(
                                       interface_cast<ISurfaceComposerClient>(client)),
-- 
cgit v1.2.3-59-g8ed1b