#146 flock: move task data off the caller stack (UAF on cancel)#8
Merged
Merged
Conversation
php_stdiop_flock_task_data_t was on the caller's stack while the libuv worker held a pointer to it. On AsyncCancellation mid-suspend the caller frame unwound and the still-running worker wrote into a freed stack slot (stack-use-after-return). Allocate the task with inline-tail storage (ZEND_ASYNC_NEW_TASK_EX), so flock_data lives at the tail of the task struct itself. The task is refcounted and survives caller unwind; libuv_task_dispose's pefree(task) frees the inline data on its own. Found via ext/async/fuzzy-tests/io/flock_chaos.feature; the cancel-mid-flock scenarios SEGV'd instantly on ASAN-ZTS. After this fix they pass on fifo + 4 random seeds plus ASAN-ZTS; full ext/async/tests/io suite (84 phpt) stays green. Fixes true-async/php-async#146
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bug
`php_stdiop_flock_task_data_t` was on the caller's stack while the libuv worker thread held a pointer to it. On AsyncCancellation mid-suspend the caller frame unwound and the still-running worker wrote into a freed stack slot. ASAN traced it precisely:
```
==…==ERROR: AddressSanitizer: stack-use-after-return
WRITE of size 4 in php_stdiop_flock_task_run main/streams/plain_wrapper.c:1097 (thread T4 = libuv worker)
```
Reproducible with ~20 lines (see php#146).
Fix
Allocate the task with inline-tail storage via the existing `ZEND_ASYNC_NEW_TASK_EX(run, data, extra_size)` API — `flock_data` lives at the tail of the task struct itself. The task is refcounted and survives caller unwind; `libuv_task_dispose`'s `pefree(task)` frees the inline data as part of normal cleanup.
No ABI changes, no new helpers — uses existing public task API.
Audit
Grepped for other `ZEND_ASYNC_NEW_TASK*` callsites in core: only this one. No similar stack-data-into-async-worker patterns elsewhere.
Test
Fixes php#146