Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit e376b3dd authored by Shruti Bihani's avatar Shruti Bihani
Browse files

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
Change-Id: I41dde165a5eba151c958b81417d9e1065af1b411
Merged-In: I41dde165a5eba151c958b81417d9e1065af1b411
(cherry picked from commit 50bf46a3)
parent 5e34d379

media/mtp/MtpFfsHandle.cpp

+14 −0
Original line number Diff line number Diff line
@@ -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

media/mtp/MtpFfsHandle.h

+4 −0
Original line number Diff line number Diff line
@@ -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;