mCallback defined in BringYourOwnSection should be an object instead of a reference.

The referenced object could be destroyed and result in native crash when
mCallback is used.

Bug: 156536687
Test: manual test with registering a section from an app
Change-Id: Ie36c0e6e64be1246539f12999f037c24377686dd

4 files changed, 10 insertions, 7 deletions

diff --git a/cmds/incidentd/src/IncidentService.cpp b/cmds/incidentd/src/IncidentService.cpp
index 9e6d0a23de10..dc1612575f38 100644
--- a/cmds/incidentd/src/IncidentService.cpp
+++ b/cmds/incidentd/src/IncidentService.cpp
@@ -351,9 +351,9 @@ Status IncidentService::reportIncidentToDumpstate(unique_fd stream,
 
 Status IncidentService::registerSection(const int id, const String16& name16,
         const sp<IIncidentDumpCallback>& callback) {
-    const char* name = String8(name16).c_str();
+    const String8 name = String8(name16);
     const uid_t callingUid = IPCThreadState::self()->getCallingUid();
-    ALOGI("Uid %d registers section %d '%s'", callingUid, id, name);
+    ALOGI("Uid %d registers section %d '%s'", callingUid, id, name.c_str());
     if (callback == nullptr) {
         return Status::fromExceptionCode(Status::EX_NULL_POINTER);
     }
@@ -363,11 +363,11 @@ Status IncidentService::registerSection(const int id, const String16& name16,
                 ALOGW("Error registering section %d: calling uid does not match", id);
                 return Status::fromExceptionCode(Status::EX_SECURITY);
             }
-            mRegisteredSections.at(i) = new BringYourOwnSection(id, name, callingUid, callback);
+            mRegisteredSections.at(i) = new BringYourOwnSection(id, name.c_str(), callingUid, callback);
             return Status::ok();
         }
     }
-    mRegisteredSections.push_back(new BringYourOwnSection(id, name, callingUid, callback));
+    mRegisteredSections.push_back(new BringYourOwnSection(id, name.c_str(), callingUid, callback));
     return Status::ok();
 }
 
diff --git a/cmds/incidentd/src/Section.cpp b/cmds/incidentd/src/Section.cpp
index 114cbb8d6460..61e5eb07130c 100644
--- a/cmds/incidentd/src/Section.cpp
+++ b/cmds/incidentd/src/Section.cpp
@@ -876,7 +876,9 @@ BringYourOwnSection::~BringYourOwnSection() {}
 
 status_t BringYourOwnSection::BlockingCall(unique_fd& pipeWriteFd) const {
     android::os::ParcelFileDescriptor pfd(std::move(pipeWriteFd));
-    mCallback->onDumpSection(pfd);
+    if(mCallback != nullptr) {
+        mCallback->onDumpSection(pfd);
+    }
     return NO_ERROR;
 }
 
diff --git a/cmds/incidentd/src/Section.h b/cmds/incidentd/src/Section.h
index 2ce45ed66a32..bc4909dcfa2e 100644
--- a/cmds/incidentd/src/Section.h
+++ b/cmds/incidentd/src/Section.h
@@ -207,7 +207,7 @@ public:
     virtual status_t BlockingCall(unique_fd& pipeWriteFd) const;
 
 private:
-    const sp<IIncidentDumpCallback>& mCallback;
+    const sp<IIncidentDumpCallback> mCallback;
 };
 
 
diff --git a/core/java/android/os/IIncidentDumpCallback.aidl b/core/java/android/os/IIncidentDumpCallback.aidl
index 09b5b01367c1..d94df34f6be9 100644
--- a/core/java/android/os/IIncidentDumpCallback.aidl
+++ b/core/java/android/os/IIncidentDumpCallback.aidl
@@ -25,7 +25,8 @@ import android.os.ParcelFileDescriptor;
  */
 oneway interface IIncidentDumpCallback {
     /**
-     * Dumps section data to the given ParcelFileDescriptor.
+     * Dumps section data to the given ParcelFileDescriptor, which needs to be
+     * closed properly after writing the data.
      */
     void onDumpSection(in ParcelFileDescriptor fd);
 }