Skip to content

fix use-after-free of a blackboard entry held by getAnyLocked - #1181

Open
aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:blackboard-locked-entry-uaf
Open

fix use-after-free of a blackboard entry held by getAnyLocked#1181
aysha-afrah26 wants to merge 1 commit into
BehaviorTree:masterfrom
aysha-afrah26:blackboard-locked-entry-uaf

Conversation

@aysha-afrah26

Copy link
Copy Markdown
Contributor

Blackboard::getAnyLocked builds the returned AnyPtrLocked from a shared_ptr<Entry> that is a local variable, so the caller ends up holding a raw Any* and a raw std::mutex* into an entry that nothing keeps alive. unset() and clear() take storage_mutex_ but never entry_mutex, so erasing the key drops the last reference and destroys the entry while its mutex is still locked, which makes the next read through the pointer a heap-use-after-free and leaves ~LockedPtr unlocking a mutex that is already gone. The path I ran into it on is the Groot2 server thread, where ExportBlackboardToJSON calls getAnyLocked per key while an UnsetBlackboard action on the tick thread frees the entry underneath it; the test added here does the same sequence on a single thread and trips ASan on current master.

Giving LockedPtr an optional shared owner and passing it the shared_ptr<Entry> keeps the entry alive for exactly as long as the lock is held, which is what getInputStamped already does when it takes entry_mutex directly. Keeping it inside LockedPtr rather than at each call site means the guarantee holds for every user of the locked-pointer API instead of depending on each one to remember. One note for review: LockedPtr grows by one shared_ptr, and it only ever lives as a local scope guard so no class layout changes, but it is a size change in a public header.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant