Migrate IDisplayEventConnection interface to AIDL

This addresses security vulnerabilities due to hard coded binder
interface.

Bug: 195660647
Test: atest services/surfaceflinger/tests/unittests/SchedulerTest.cpp
Change-Id: I948e97e37056286d54623ca6232580187b138e62

15 files changed, 53 insertions, 136 deletions

diff --git a/libs/binder/include/binder/IInterface.h b/libs/binder/include/binder/IInterface.h
index 415b19adf6..f5abb859bc 100644
--- a/libs/binder/include/binder/IInterface.h
+++ b/libs/binder/include/binder/IInterface.h
@@ -228,7 +228,6 @@ constexpr const char* const kManualInterfaces[] = {
   "android.gfx.tests.IIPCTest",
   "android.gfx.tests.ISafeInterfaceTest",
   "android.graphicsenv.IGpuService",
-  "android.gui.DisplayEventConnection",
   "android.gui.IConsumerListener",
   "android.gui.IGraphicBufferConsumer",
   "android.gui.ITransactionComposerListener",
diff --git a/libs/gui/Android.bp b/libs/gui/Android.bp
index fea9f810c2..c5dec19582 100644
--- a/libs/gui/Android.bp
+++ b/libs/gui/Android.bp
@@ -180,7 +180,6 @@ cc_library_shared {
         "FrameTimelineInfo.cpp",
         "GLConsumer.cpp",
         "IConsumerListener.cpp",
-        "IDisplayEventConnection.cpp",
         "IGraphicBufferConsumer.cpp",
         "IGraphicBufferProducer.cpp",
         "IProducerListener.cpp",
diff --git a/libs/gui/DisplayEventReceiver.cpp b/libs/gui/DisplayEventReceiver.cpp
index 03b33c7330..b916e48f79 100644
--- a/libs/gui/DisplayEventReceiver.cpp
+++ b/libs/gui/DisplayEventReceiver.cpp
@@ -19,7 +19,6 @@
 #include <utils/Errors.h>
 
 #include <gui/DisplayEventReceiver.h>
-#include <gui/IDisplayEventConnection.h>
 #include <gui/ISurfaceComposer.h>
 
 #include <private/gui/ComposerService.h>
diff --git a/libs/gui/IDisplayEventConnection.cpp b/libs/gui/IDisplayEventConnection.cpp
deleted file mode 100644
index c0e246fa15..0000000000
--- a/libs/gui/IDisplayEventConnection.cpp
+++ /dev/null
@@ -1,80 +0,0 @@
-/*
- * Copyright (C) 2011 The Android Open Source Project
- *
- * Licensed under the Apache License, Version 2.0 (the "License");
- * you may not use this file except in compliance with the License.
- * You may obtain a copy of the License at
- *
- *      http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-
-#include <gui/IDisplayEventConnection.h>
-
-#include <private/gui/BitTube.h>
-
-namespace android {
-
-namespace { // Anonymous
-
-enum class Tag : uint32_t {
-    STEAL_RECEIVE_CHANNEL = IBinder::FIRST_CALL_TRANSACTION,
-    SET_VSYNC_RATE,
-    REQUEST_NEXT_VSYNC,
-    LAST = REQUEST_NEXT_VSYNC,
-};
-
-} // Anonymous namespace
-
-class BpDisplayEventConnection : public SafeBpInterface<IDisplayEventConnection> {
-public:
-    explicit BpDisplayEventConnection(const sp<IBinder>& impl)
-          : SafeBpInterface<IDisplayEventConnection>(impl, "BpDisplayEventConnection") {}
-
-    ~BpDisplayEventConnection() override;
-
-    status_t stealReceiveChannel(gui::BitTube* outChannel) override {
-        return callRemote<decltype(
-                &IDisplayEventConnection::stealReceiveChannel)>(Tag::STEAL_RECEIVE_CHANNEL,
-                                                                outChannel);
-    }
-
-    status_t setVsyncRate(uint32_t count) override {
-        return callRemote<decltype(&IDisplayEventConnection::setVsyncRate)>(Tag::SET_VSYNC_RATE,
-                                                                            count);
-    }
-
-    void requestNextVsync() override {
-        callRemoteAsync<decltype(&IDisplayEventConnection::requestNextVsync)>(
-                Tag::REQUEST_NEXT_VSYNC);
-    }
-};
-
-// Out-of-line virtual method definition to trigger vtable emission in this translation unit (see
-// clang warning -Wweak-vtables)
-BpDisplayEventConnection::~BpDisplayEventConnection() = default;
-
-IMPLEMENT_META_INTERFACE(DisplayEventConnection, "android.gui.DisplayEventConnection");
-
-status_t BnDisplayEventConnection::onTransact(uint32_t code, const Parcel& data, Parcel* reply,
-                                              uint32_t flags) {
-    if (code < IBinder::FIRST_CALL_TRANSACTION || code > static_cast<uint32_t>(Tag::LAST)) {
-        return BBinder::onTransact(code, data, reply, flags);
-    }
-    auto tag = static_cast<Tag>(code);
-    switch (tag) {
-        case Tag::STEAL_RECEIVE_CHANNEL:
-            return callLocal(data, reply, &IDisplayEventConnection::stealReceiveChannel);
-        case Tag::SET_VSYNC_RATE:
-            return callLocal(data, reply, &IDisplayEventConnection::setVsyncRate);
-        case Tag::REQUEST_NEXT_VSYNC:
-            return callLocalAsync(data, reply, &IDisplayEventConnection::requestNextVsync);
-    }
-}
-
-} // namespace android
diff --git a/libs/gui/ISurfaceComposer.cpp b/libs/gui/ISurfaceComposer.cpp
index a90f7559b3..7f73013f6e 100644
--- a/libs/gui/ISurfaceComposer.cpp
+++ b/libs/gui/ISurfaceComposer.cpp
@@ -17,12 +17,12 @@
 // tag as surfaceflinger
 #define LOG_TAG "SurfaceFlinger"
 
+#include <android/gui/IDisplayEventConnection.h>
 #include <android/gui/IRegionSamplingListener.h>
 #include <android/gui/ITransactionTraceListener.h>
 #include <binder/IPCThreadState.h>
 #include <binder/IServiceManager.h>
 #include <binder/Parcel.h>
-#include <gui/IDisplayEventConnection.h>
 #include <gui/IGraphicBufferProducer.h>
 #include <gui/ISurfaceComposer.h>
 #include <gui/ISurfaceComposerClient.h>
@@ -44,6 +44,7 @@
 
 namespace android {
 
+using gui::IDisplayEventConnection;
 using gui::IRegionSamplingListener;
 using gui::IWindowInfosListener;
 using ui::ColorMode;
diff --git a/libs/gui/aidl/android/gui/BitTube.aidl b/libs/gui/aidl/android/gui/BitTube.aidl
new file mode 100644
index 0000000000..6b0595ec66
--- /dev/null
+++ b/libs/gui/aidl/android/gui/BitTube.aidl
@@ -0,0 +1,19 @@
+/*
+ * Copyright 2021 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package android.gui;
+
+parcelable BitTube cpp_header "private/gui/BitTube.h";
diff --git a/libs/gui/include/gui/IDisplayEventConnection.h b/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl
index cff22a368a..9f41593539 100644
--- a/libs/gui/include/gui/IDisplayEventConnection.h
+++ b/libs/gui/aidl/android/gui/IDisplayEventConnection.aidl
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2011 The Android Open Source Project
+ * Copyright 2021 The Android Open Source Project
  *
  * Licensed under the Apache License, Version 2.0 (the "License");
  * you may not use this file except in compliance with the License.
@@ -14,52 +14,28 @@
  * limitations under the License.
  */
 
-#pragma once
+package android.gui;
 
-#include <binder/IInterface.h>
-#include <binder/SafeInterface.h>
-#include <gui/ISurfaceComposer.h>
-#include <utils/Errors.h>
-
-#include <cstdint>
-
-namespace android {
-
-namespace gui {
-class BitTube;
-} // namespace gui
-
-class IDisplayEventConnection : public IInterface {
-public:
-    DECLARE_META_INTERFACE(DisplayEventConnection)
+import android.gui.BitTube;
 
+/** @hide */
+interface IDisplayEventConnection {
     /*
      * stealReceiveChannel() returns a BitTube to receive events from. Only the receive file
      * descriptor of outChannel will be initialized, and this effectively "steals" the receive
      * channel from the remote end (such that the remote end can only use its send channel).
      */
-    virtual status_t stealReceiveChannel(gui::BitTube* outChannel) = 0;
+    void stealReceiveChannel(out BitTube outChannel);
 
     /*
      * setVsyncRate() sets the vsync event delivery rate. A value of 1 returns every vsync event.
      * A value of 2 returns every other event, etc. A value of 0 returns no event unless
      * requestNextVsync() has been called.
      */
-    virtual status_t setVsyncRate(uint32_t count) = 0;
+    void setVsyncRate(in int count);
 
     /*
      * requestNextVsync() schedules the next vsync event. It has no effect if the vsync rate is > 0.
      */
-    virtual void requestNextVsync() = 0; // Asynchronous
-};
-
-class BnDisplayEventConnection : public SafeBnInterface<IDisplayEventConnection> {
-public:
-    BnDisplayEventConnection()
-          : SafeBnInterface<IDisplayEventConnection>("BnDisplayEventConnection") {}
-
-    status_t onTransact(uint32_t code, const Parcel& data, Parcel* reply,
-                        uint32_t flags = 0) override;
-};
-
-} // namespace android
+    oneway void requestNextVsync(); // Asynchronous
+}
diff --git a/libs/gui/include/gui/DisplayEventReceiver.h b/libs/gui/include/gui/DisplayEventReceiver.h
index ca368433b1..456bbfb611 100644
--- a/libs/gui/include/gui/DisplayEventReceiver.h
+++ b/libs/gui/include/gui/DisplayEventReceiver.h
@@ -33,7 +33,7 @@ namespace android {
 
 // ----------------------------------------------------------------------------
 
-class IDisplayEventConnection;
+using gui::IDisplayEventConnection;
 
 namespace gui {
 class BitTube;
diff --git a/libs/gui/include/gui/ISurfaceComposer.h b/libs/gui/include/gui/ISurfaceComposer.h
index e1e9182056..2546e4c753 100644
--- a/libs/gui/include/gui/ISurfaceComposer.h
+++ b/libs/gui/include/gui/ISurfaceComposer.h
@@ -17,6 +17,7 @@
 #pragma once
 
 #include <android/gui/DisplayBrightness.h>
+#include <android/gui/IDisplayEventConnection.h>
 #include <android/gui/IFpsListener.h>
 #include <android/gui/IHdrLayerInfoListener.h>
 #include <android/gui/IRegionSamplingListener.h>
@@ -61,12 +62,12 @@ struct InputWindowCommands;
 struct LayerCaptureArgs;
 class LayerDebugInfo;
 class HdrCapabilities;
-class IDisplayEventConnection;
 class IGraphicBufferProducer;
 class ISurfaceComposerClient;
 class Rect;
 enum class FrameEvent;
 
+using gui::IDisplayEventConnection;
 using gui::IRegionSamplingListener;
 using gui::IScreenCaptureListener;
 
diff --git a/libs/gui/tests/Surface_test.cpp b/libs/gui/tests/Surface_test.cpp
index e0c2aa7181..d6ac3f9745 100644
--- a/libs/gui/tests/Surface_test.cpp
+++ b/libs/gui/tests/Surface_test.cpp
@@ -19,11 +19,11 @@
 #include <gtest/gtest.h>
 
 #include <SurfaceFlingerProperties.h>
+#include <android/gui/IDisplayEventConnection.h>
 #include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h>
 #include <binder/ProcessState.h>
 #include <configstore/Utils.h>
 #include <gui/BufferItemConsumer.h>
-#include <gui/IDisplayEventConnection.h>
 #include <gui/IProducerListener.h>
 #include <gui/ISurfaceComposer.h>
 #include <gui/Surface.h>
@@ -45,6 +45,7 @@ using namespace std::chrono_literals;
 // retrieve wide-color and hdr settings from configstore
 using namespace android::hardware::configstore;
 using namespace android::hardware::configstore::V1_0;
+using gui::IDisplayEventConnection;
 using gui::IRegionSamplingListener;
 using ui::ColorMode;
 
diff --git a/services/surfaceflinger/Scheduler/EventThread.cpp b/services/surfaceflinger/Scheduler/EventThread.cpp
index 455289ff96..627c49a853 100644
--- a/services/surfaceflinger/Scheduler/EventThread.cpp
+++ b/services/surfaceflinger/Scheduler/EventThread.cpp
@@ -170,20 +170,21 @@ void EventThreadConnection::onFirstRef() {
     mEventThread->registerDisplayEventConnection(this);
 }
 
-status_t EventThreadConnection::stealReceiveChannel(gui::BitTube* outChannel) {
+binder::Status EventThreadConnection::stealReceiveChannel(gui::BitTube* outChannel) {
     outChannel->setReceiveFd(mChannel.moveReceiveFd());
     outChannel->setSendFd(base::unique_fd(dup(mChannel.getSendFd())));
-    return NO_ERROR;
+    return binder::Status::ok();
 }
 
-status_t EventThreadConnection::setVsyncRate(uint32_t rate) {
-    mEventThread->setVsyncRate(rate, this);
-    return NO_ERROR;
+binder::Status EventThreadConnection::setVsyncRate(int rate) {
+    mEventThread->setVsyncRate(static_cast<uint32_t>(rate), this);
+    return binder::Status::ok();
 }
 
-void EventThreadConnection::requestNextVsync() {
+binder::Status EventThreadConnection::requestNextVsync() {
     ATRACE_NAME("requestNextVsync");
     mEventThread->requestNextVsync(this);
+    return binder::Status::ok();
 }
 
 status_t EventThreadConnection::postEvent(const DisplayEventReceiver::Event& event) {
diff --git a/services/surfaceflinger/Scheduler/EventThread.h b/services/surfaceflinger/Scheduler/EventThread.h
index de435708a6..fa9af098f6 100644
--- a/services/surfaceflinger/Scheduler/EventThread.h
+++ b/services/surfaceflinger/Scheduler/EventThread.h
@@ -17,8 +17,8 @@
 #pragma once
 
 #include <android-base/thread_annotations.h>
+#include <android/gui/BnDisplayEventConnection.h>
 #include <gui/DisplayEventReceiver.h>
-#include <gui/IDisplayEventConnection.h>
 #include <private/gui/BitTube.h>
 #include <sys/types.h>
 #include <utils/Errors.h>
@@ -80,7 +80,7 @@ public:
     virtual void dump(std::string& result) const = 0;
 };
 
-class EventThreadConnection : public BnDisplayEventConnection {
+class EventThreadConnection : public gui::BnDisplayEventConnection {
 public:
     EventThreadConnection(EventThread*, uid_t callingUid, ResyncCallback,
                           ISurfaceComposer::EventRegistrationFlags eventRegistration = {});
@@ -88,9 +88,9 @@ public:
 
     virtual status_t postEvent(const DisplayEventReceiver::Event& event);
 
-    status_t stealReceiveChannel(gui::BitTube* outChannel) override;
-    status_t setVsyncRate(uint32_t rate) override;
-    void requestNextVsync() override; // asynchronous
+    binder::Status stealReceiveChannel(gui::BitTube* outChannel) override;
+    binder::Status setVsyncRate(int rate) override;
+    binder::Status requestNextVsync() override; // asynchronous
 
     // Called in response to requestNextVsync.
     const ResyncCallback resyncCallback;
diff --git a/services/surfaceflinger/Scheduler/MessageQueue.h b/services/surfaceflinger/Scheduler/MessageQueue.h
index dd69d60580..9532e26a9c 100644
--- a/services/surfaceflinger/Scheduler/MessageQueue.h
+++ b/services/surfaceflinger/Scheduler/MessageQueue.h
@@ -22,7 +22,7 @@
 #include <utility>
 
 #include <android-base/thread_annotations.h>
-#include <gui/IDisplayEventConnection.h>
+#include <android/gui/IDisplayEventConnection.h>
 #include <private/gui/BitTube.h>
 #include <utils/Looper.h>
 #include <utils/Timers.h>
diff --git a/services/surfaceflinger/SurfaceFlinger.cpp b/services/surfaceflinger/SurfaceFlinger.cpp
index 14761ea7ea..08d85275fe 100644
--- a/services/surfaceflinger/SurfaceFlinger.cpp
+++ b/services/surfaceflinger/SurfaceFlinger.cpp
@@ -26,6 +26,7 @@
 
 #include <android-base/properties.h>
 #include <android/configuration.h>
+#include <android/gui/IDisplayEventConnection.h>
 #include <android/hardware/configstore/1.0/ISurfaceFlingerConfigs.h>
 #include <android/hardware/configstore/1.1/ISurfaceFlingerConfigs.h>
 #include <android/hardware/configstore/1.1/types.h>
@@ -51,7 +52,6 @@
 #include <ftl/future.h>
 #include <gui/BufferQueue.h>
 #include <gui/DebugEGLImageTracker.h>
-#include <gui/IDisplayEventConnection.h>
 #include <gui/IProducerListener.h>
 #include <gui/LayerDebugInfo.h>
 #include <gui/LayerMetadata.h>
@@ -169,6 +169,7 @@ using namespace android::sysprop;
 using android::hardware::power::Boost;
 using base::StringAppendF;
 using gui::DisplayInfo;
+using gui::IDisplayEventConnection;
 using gui::IWindowInfosListener;
 using gui::WindowInfo;
 using ui::ColorMode;
diff --git a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
index a6fd378d3d..f48abb7519 100644
--- a/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
+++ b/services/surfaceflinger/tests/unittests/SchedulerTest.cpp
@@ -48,9 +48,9 @@ protected:
               : EventThreadConnection(eventThread, /*callingUid=*/0, ResyncCallback()) {}
         ~MockEventThreadConnection() = default;
 
-        MOCK_METHOD1(stealReceiveChannel, status_t(gui::BitTube* outChannel));
-        MOCK_METHOD1(setVsyncRate, status_t(uint32_t count));
-        MOCK_METHOD0(requestNextVsync, void());
+        MOCK_METHOD1(stealReceiveChannel, binder::Status(gui::BitTube* outChannel));
+        MOCK_METHOD1(setVsyncRate, binder::Status(int count));
+        MOCK_METHOD0(requestNextVsync, binder::Status());
     };
 
     SchedulerTest();