CVE-2024-34731 AI-generated analysis
platform/frameworks/av (media/module/libmediatranscoding TranscodingResourcePolicy); platform/hardware/interfaces (health/aidl/default Health/LinkedCallback, neuralnetworks/aidl/utils ProtectCallback DeathMonitor); platform/system/security (keystore2/apc_compat ConfuiAidlCompatSession)
Replaces raw object pointers used as libbinder_ndk death-recipient cookies with tracked, validated cookie objects (keyed maps, weak_ptr wrappers, onUnlinked-owned cookies) in libmediatranscoding, the health AIDL HAL default impl, the neuralnetworks AIDL utils and keystore2 apc_compat, so a late binderDied callback no longer dereferences a freed object.
Root cause
In all four components the cookie passed to AIBinder_linkToDeath was a raw pointer to an object: `this` for TranscodingResourcePolicy and ConfuiAidlCompatSession, a raw LinkedCallback* in the health HAL and a raw DeathMonitor* in NNAPI utils. The death callbacks (TranscodingResourcePolicy::BinderDiedCallback, OnCallbackDiedWrapped, DeathMonitor::serviceDied(void*), binderDiedCallbackAidl) cast it back and used it. At most they checked for null; none checked that the object was still alive. The health commit message says an onBinderDied callback can still arrive after AIBinder_unlinkToDeath(). In the health HAL, the LinkedCallback destructor unlinked and then freed the very object that a pending callback would use. Nothing validated that the cookie still referred to a live object, so the callback could operate on freed memory.
How the fix works
- frameworks/av media/module/libmediatranscoding/TranscodingResourcePolicy.cpp: adds TranscodingResourcePolicyCookie and a file-static std::map<uintptr_t, unique_ptr<cookie>> sCookies, with sCookieKeyCounter and sCookiesMutex. registerSelf() now allocates a cookie, assigns it an integer key and stores it in sCookies. It also records the key in the per-instance mCookieKeys set, guarded by the new mCookieKeysLock (both declared in include/media/TranscodingResourcePolicy.h). It then passes the integer key, not `this`, to AIBinder_linkToDeath.
- frameworks/av TranscodingResourcePolicy::BinderDiedCallback: takes sCookiesMutex and looks the key up in sCookies. Only if it is found does it call policy->unregisterSelf() and erase the entry; an unknown key is ignored. ~TranscodingResourcePolicy(): under sCookiesMutex, erases every sCookies entry whose key is in mCookieKeys before calling unregisterSelf(), so a later death notification finds nothing. unregisterSelf(): the AIBinder_unlinkToDeath call is removed. Per the commit message, dropping the last strong ref to the binder unlinks it.
- hardware/interfaces health/aidl/default/Health.cpp: adds onCallbackUnlinked(void*), which deletes the LinkedCallback, and registers it with AIBinder_DeathRecipient_setOnUnlinked in Health::Health. Ownership of the cookie moves to the death recipient, so it is freed in the onUnlinked callback. callbacks_ (include/health-impl/Health.h) changes from vector<unique_ptr<LinkedCallback>> to map<LinkedCallback*, shared_ptr<IHealthInfoCallback>>. registerCallback returns STATUS_UNEXPECTED_NULL if LinkedCallback::Make fails. unregisterCallback calls AIBinder_unlinkToDeath explicitly, with the map key as the cookie, and erases the entry. OnHealthInfoChanged iterates the map and erases dead entries.
- hardware/interfaces health/aidl/default/LinkedCallback.cpp/.h: Make returns a raw LinkedCallback*, with ownership passed to the death recipient. The destructor that called AIBinder_unlinkToDeath is removed. callback_ becomes weak_ptr<IHealthInfoCallback>, and OnCallbackDied only calls service()->unregisterCallback if the weak_ptr can be promoted.
- hardware/interfaces neuralnetworks/aidl/utils/src/ProtectCallback.cpp: adds anonymous-namespace sCookiesMutex, sCookieKeyCounter and std::map<uintptr_t, weak_ptr<DeathMonitor>> sCookies. DeathHandler::create allocates a key under the lock and constructs DeathMonitor(key), using the new constructor and kCookieKey in include/nnapi/hal/aidl/ProtectCallback.h. It registers the monitor and passes the key to AIBinder_linkToDeath; ~DeathHandler unlinks with the key. DeathMonitor::serviceDied(void*) looks the key up under the lock, erases it, promotes the weak_ptr and only then calls serviceDied(); a missing key or a failed promotion is logged and ignored. The new ~DeathMonitor erases its key under the lock, and copy/move are deleted. test/DeviceTest.cpp is updated to pass getCookieKey().
- system/security keystore2/apc_compat/apc_compat.cpp ConfuiAidlCompatSession: adds DeathRecipientCookie, which holds a weak_ptr<ConfuiAidlCompatSession>. promptUserConfirmation allocates one cookie per linkToDeath call, deletes it if linking fails, and tracks it in deathRecipientCookie_ (a std::set<void*> under deathRecipientCookie_lock_). finalize() copies the tracked cookies out and unlinks each one outside the lock. serviceDied() clears the set, and the new serviceUnlinked(cookie) erases one entry. binderDiedCallbackAidl casts to DeathRecipientCookie, promotes the weak_ptr and calls serviceDied only if the session is still alive. The new binderUnlinkedCallbackAidl, installed with AIBinder_DeathRecipient_setOnUnlinked in the constructor, calls serviceUnlinked and deletes the cookie.
Completeness
- All four death-recipient cookie sites linked to bug 319210610 are changed so that the callback validates the cookie, or safely promotes it, before touching the owner. The raw `this`/raw-pointer cookie pattern is removed from each.
- libmediatranscoding: in registerSelf(), sCookieKeyCounter++ and sCookies.emplace() run without holding sCookiesMutex; only the mCookieKeys insertion takes mCookieKeysLock. BinderDiedCallback and the destructor, by contrast, modify sCookies under sCookiesMutex. registerSelf is a private method, and the material does not show which lock its callers hold. The fix also drops AIBinder_unlinkToDeath entirely and relies on the binder reference being released in unregisterSelf.
- health HAL: OnHealthInfoChanged erases dead entries from callbacks_ without calling AIBinder_unlinkToDeath; the LinkedCallback is then freed only when the death recipient's onUnlinked fires. The health and keystore2 fixes depend on AIBinder_DeathRecipient_setOnUnlinked being available.
- NNAPI utils: serviceDied(void*) erases the key on the first notification, so each cookie is consumed once. Only the NDK path is tracked (code comment: 'Only used with ndk').
- keystore2: serviceDied() clears deathRecipientCookie_ without unlinking; the cookies for the dead binder are freed by binderUnlinkedCallbackAidl when binder delivers onUnlinked.
Behavior changes
- health Health::registerCallback now fails with STATUS_UNEXPECTED_NULL when LinkedCallback::Make (AIBinder_linkToDeath) fails, instead of storing a null entry.
- health Health::unregisterCallback logs a WARNING if AIBinder_unlinkToDeath fails with a status other than OK/DEAD_OBJECT. OnHealthInfoChanged logs at DEBUG when a callback result is neither OK nor dead.
- libmediatranscoding no longer calls AIBinder_unlinkToDeath in unregisterSelf. BinderDiedCallback logs 'BinderDiedCallback unregistering TranscodingResourcePolicy'.
- NNAPI DeathMonitor::serviceDied logs and ignores death notifications whose cookie key is unknown or whose DeathMonitor can no longer be promoted.
- keystore2 ConfuiAidlCompatSession: finalize() unlinks all cookies accumulated from multiple promptUserConfirmation calls. Death notifications that arrive after the session is gone are logged and ignored.
Per major
- Android 14: same fix: four commits (frameworks/av 4b68b0099384, hardware/interfaces 0ff19d1f8961 and d63d09261806, system/security d3805312d734) landed in android-security-14.0.0_r11. At android-security-14.0.0_r29 the system/security patched region is fully present (8/8 lines), and the r29 code context shows the av and hardware/interfaces changes in place.
- Android 15: not affected (not listed by the bulletin); at android-security-15.0.0_r17, 7/8 lines of the patched region are present.
- Android 16: not affected (not listed by the bulletin); at android-security-16.0.0_r8, 7/8 lines of the patched region are present.
- Android 17: not affected (not listed by the bulletin); at android-security-17.0.0_r1, 7/8 lines of the patched region are present.
Unknowns
- Which of the four components the bulletin's High EoP rating is attributed to, and the concrete trigger.
- Whether registerSelf() in TranscodingResourcePolicy is always invoked under a lock that serializes it against BinderDiedCallback and the destructor, given that sCookies is modified there without sCookiesMutex.
- What the one missing line of the patched region on 15/16/17 is.
- How the fix was applied on the other affected branches (12, 12L, 13), including whether AIBinder_DeathRecipient_setOnUnlinked is usable there.