Fix heap-use-after-free issue flagged by fuzzer test.

A data member of class MtpFfsHandle is being accessed after the class object has been freed in the fuzzer. The method accessing the data member is running in a separate thread that gets detached from its parent. Using a conditional variable with an atomic int predicate in the close() function to ensure the detached thread's execution has completed before freeing the object fixes the issue without blocking the processing mid-way.

Bug: 243381410
Test: Build mtp_handle_fuzzer and run on the target device
(cherry picked from commit 50bf46a3f62136386548a9187a749936bda3ee8f)
(cherry picked from https://googleplex-android-review.googlesource.com/q/commit:05f6e7b4bc7549a3160404557e6d7077733f5eb5)
Merged-In: I41dde165a5eba151c958b81417d9e1065af1b411
Change-Id: I41dde165a5eba151c958b81417d9e1065af1b411

2 files changed, 18 insertions, 0 deletions

diff --git a/media/mtp/MtpFfsHandle.cpp b/media/mtp/MtpFfsHandle.cpp
index 2ffd7759e0..ef8c9aa789 100644
--- a/media/mtp/MtpFfsHandle.cpp
+++ b/media/mtp/MtpFfsHandle.cpp
@@ -297,6 +297,10 @@ int MtpFfsHandle::start(bool ptp) {
 }
 
 void MtpFfsHandle::close() {
+    auto timeout = std::chrono::seconds(2);
+    std::unique_lock lk(m);
+    cv.wait_for(lk, timeout ,[this]{return child_threads==0;});
+
     io_destroy(mCtx);
     closeEndpoints();
     closeConfig();
@@ -669,6 +673,11 @@ int MtpFfsHandle::sendEvent(mtp_event me) {
     char *temp = new char[me.length];
     memcpy(temp, me.data, me.length);
     me.data = temp;
+
+    std::unique_lock lk(m);
+    child_threads++;
+    lk.unlock();
+
     std::thread t([this, me]() { return this->doSendEvent(me); });
     t.detach();
     return 0;
@@ -680,6 +689,11 @@ void MtpFfsHandle::doSendEvent(mtp_event me) {
     if (static_cast<unsigned>(ret) != length)
         PLOG(ERROR) << "Mtp error sending event thread!";
     delete[] reinterpret_cast<char*>(me.data);
+
+    std::unique_lock lk(m);
+    child_threads--;
+    lk.unlock();
+    cv.notify_one();
 }
 
 } // namespace android
diff --git a/media/mtp/MtpFfsHandle.h b/media/mtp/MtpFfsHandle.h
index e552e03bec..51cdef0953 100644
--- a/media/mtp/MtpFfsHandle.h
+++ b/media/mtp/MtpFfsHandle.h
@@ -60,6 +60,10 @@ protected:
     bool mCanceled;
     bool mBatchCancel;
 
+    std::mutex m;
+    std::condition_variable cv;
+    std::atomic<int> child_threads{0};
+
     android::base::unique_fd mControl;
     // "in" from the host's perspective => sink for mtp server
     android::base::unique_fd mBulkIn;