fix: avoid use-after-free of pQuery->pCmdMsg in asyncExecDdlQuery#35152
fix: avoid use-after-free of pQuery->pCmdMsg in asyncExecDdlQuery#35152guanshengliang merged 1 commit intomainfrom
Conversation
asyncSendMsgToServer() is non-blocking. Once it returns, the async response callback on another thread may already have called doDestroyRequest() -> nodesDestroyAllocator(), freeing the chunk that contains SQuery. Any subsequent access to pQuery (including reading pQuery->pCmdMsg for the taosMemoryFreeClear call) is a use-after-free. Fix: save pQuery->pCmdMsg into a local variable (pMsgInfo), then set pQuery->pCmdMsg = NULL before the async call so that nodesDestroyNode() — if triggered by the callback — sees NULL and skips the free. After asyncSendMsgToServer() returns, free via the local pMsgInfo to avoid touching the potentially-freed pQuery. Reproducer: loop test_scalar_sub4a2.py triggers the race via the DDL path; ASAN reports heap-use-after-free at clientImpl.c:520. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Fixes a race in the client async DDL execution path where asyncSendMsgToServer() can return after the async callback has already freed the allocator chunk containing SQuery, leading to a use-after-free when accessing pQuery->pCmdMsg.
Changes:
- Saves
pQuery->pCmdMsginto a localpMsgInfoand clearspQuery->pCmdMsgbefore the async send to prevent concurrent destructor frees. - Frees the
SCmdMsgInfovia the local variable after the async send to avoid touching potentially freedpQuery.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Code Review
This pull request updates the asyncExecDdlQuery function in clientImpl.c to prevent potential race conditions and double-free errors. By clearing pQuery->pCmdMsg before the asynchronous call and using a local pointer for memory deallocation, the code ensures thread safety in cases where the query object might be destroyed by a callback on another thread. I have no feedback to provide.
asyncSendMsgToServer() is non-blocking. Once it returns, the async response callback on another thread may already have called doDestroyRequest() -> nodesDestroyAllocator(), freeing the chunk that contains SQuery. Any subsequent access to pQuery (including reading pQuery->pCmdMsg for the taosMemoryFreeClear call) is a use-after-free.
Fix: save pQuery->pCmdMsg into a local variable (pMsgInfo), then set pQuery->pCmdMsg = NULL before the async call so that nodesDestroyNode() — if triggered by the callback — sees NULL and skips the free. After asyncSendMsgToServer() returns, free via the local pMsgInfo to avoid touching the potentially-freed pQuery.
Reproducer: loop test_scalar_sub4a2.py triggers the race via the DDL path; ASAN reports heap-use-after-free at clientImpl.c:520.
Description
Issue(s)
Checklist
Please check the items in the checklist if applicable.