CVE-2023-40114 AI-generated analysis
platform/frameworks/av, media/mtp (MtpFfsHandle, FunctionFS MTP transport)
Adds a mutex, condition variable and atomic child-thread counter to MtpFfsHandle so that close() waits (up to 2 seconds) for detached doSendEvent threads spawned by sendEvent() to finish before the handle is torn down, preventing a heap use-after-free of the handle's members from the detached thread.
Root cause
MtpFfsHandle::sendEvent() copies the event payload and launches a std::thread running this->doSendEvent(me), then immediately detaches it. Nothing tracked that thread's lifetime, so MtpFfsHandle::close() and the subsequent freeing of the MtpFfsHandle object could proceed while doSendEvent was still running and still accessing data members of the object (the commit message says a data member was accessed after the object was freed, as found by the mtp_handle_fuzzer).
How the fix works
- media/mtp/MtpFfsHandle.h: adds three protected members: std::mutex m, std::condition_variable cv, and std::atomic<int> child_threads{0} to count outstanding detached event-sender threads.
- media/mtp/MtpFfsHandle.cpp:MtpFfsHandle::sendEvent: after copying the event data and before creating/detaching the std::thread, takes unique_lock on m, increments child_threads, and unlocks.
- media/mtp/MtpFfsHandle.cpp:MtpFfsHandle::doSendEvent: at the end of the function (after the send result check and delete[] of the copied data), takes the lock, decrements child_threads, unlocks, and calls cv.notify_one() so a waiting close() can proceed.
- media/mtp/MtpFfsHandle.cpp:MtpFfsHandle::close: before io_destroy(mCtx), closeEndpoints() and closeConfig(), takes unique_lock on m and calls cv.wait_for(lk, 2 seconds, predicate child_threads==0), so teardown is delayed until all in-flight doSendEvent threads have completed or the timeout expires.
Completeness
- Covers the sendEvent/doSendEvent detached thread, which is the only std::thread creation visible in the diff; both the increment (before detach) and decrement (end of doSendEvent) are under the mutex.
- The wait in close() is bounded by a 2 second timeout: if a doSendEvent thread is still running past the timeout, close() continues and teardown while the thread is live (including the thread's own later child_threads-- on the object) is not prevented; the fix narrows the window rather than eliminating it.
- Synchronization is only in close(); the diff does not add anything to the destructor, so an MtpFfsHandle destroyed without close() being called first gets no protection (whether the destructor calls close() is not shown in the material).
- The fix does not change doSendEvent to avoid touching this; it only orders teardown after the thread.
Behavior changes
- MtpFfsHandle::close() may now block for up to 2 seconds while an event-send thread is outstanding.
- sendEvent() and doSendEvent() take a mutex briefly; no change to return values or error codes.
- Adds std::mutex, std::condition_variable and std::atomic<int> members to MtpFfsHandle (object layout change; code using the header must be rebuilt).
Per major
- Android 14: same fix: commit 1deb1302cfcc in android-security-14.0.0_r2 (cherry-pick of 50bf46a3f621, Change-Id I41dde165a5eba151c958b81417d9e1065af1b411); all 8 patched lines present at android-security-14.0.0_r29.
- Android 15: not affected per bulletin; patched code (8/8 lines) present at android-security-15.0.0_r17.
- Android 16: not affected per bulletin; patched code (8/8 lines) present at android-security-16.0.0_r8.
- Android 17: not affected per bulletin; patched code (8/8 lines) present at android-security-17.0.0_r1.
Unknowns
- Which specific data member doSendEvent accessed after free (the commit message says a data member, the diff does not identify it).
- Which endpoint and write call doSendEvent uses (not shown in the material).
- Whether ~MtpFfsHandle calls close(), and therefore whether destruction paths that skip close() are protected.
- Whether a 2 second timeout is sufficient in practice for the event send to complete.
- How the patched code reached the 15, 16 and 17 branches.
- Realistic (non-fuzzer) trigger conditions and the privilege boundary behind the High EoP rating are not described in the material.