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

1 files changed, 19 insertions, 8 deletions

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)),