Repository navigation
Conversation
…php#22493) phpdbg_add_watch_element() frees the passed element and returns the existing one when the variable is already watched, but several callers kept using the freed pointer, so re-watching a variable was a use-after-free in more places than the two reported. Report the duplicate back through the parse callback and register nothing, and reject a recursive watch's hashtable element that collides with an existing watch, at creation and recreation, rather than freeing a still-referenced one. A recursive watch over an already-array-watched variable now watches only the root and skips the shared hashtable. Running the tests under ASAN exposed a second, latent use-after-free at shutdown: the watch tables are module-scoped but hold request-pool memory, so under the tracked allocator freeing a watched variable during executor teardown re-queues an element after the last drain, tracked_free_all frees the pool, and phpdbg_destroy_watchpoints reads it at module shutdown. Tear the watch state down in a post_deactivate handler, which runs after the executor frees values but before the memory manager is torn down. Dropping watch_006's xfail surfaced a separate pre-existing bug on large-page platforms: the fault handler rechecks every watchpoint on the faulting page, so on 16 KB pages a value-only write to an element can fault the page holding a watched hashtable's own struct and run the recursive rescan, which re-reported existing elements as freshly added. Only run that rescan when the element count actually changed. Fixes phpGH-22480 (cherry picked from commit 207dd1d)
phpdbg watched every array element as a Bucket, but packed arrays store bare zvals and de-indirected stack variables aren't buckets, so reading Bucket.h/.key over-read the neighbour and a sibling write tripped a phantom break. Watch such elements as WATCH_ON_ZVAL and compare the type info first, reading the value union only for initialised slots so an uninitialised CV is never compared by value. A separate pre-existing bug crashed 32-bit: phpdbg_btree_insert_or_update linked each new node into the tree before initialising its child pointers, so a write to a node on a watched page faulted into the watchpoint handler, which walked the half-built node. Build the node fully, then publish it with a single store. Closes phpGH-22756 (cherry picked from commit 250f138)
PHPDBG_G(watch_recreation) is keyed by element->str, so a watch element and
its implicit parent are stored under distinct keys ("$lower[0]" and
"$lower[]"). The deduplication in phpdbg_queue_element_for_recreation() only
walks down from the entry found under the *same* key, so it never notices
that the two belong to the same chain and both get queued.
phpdbg_free_watch_element_tree() frees the element together with its entire
parent and child chains, so draining watch_recreation freed that chain twice:
the first entry frees the whole chain, the second one then walks the already
freed links and frees them again.
Drop every entry referencing a member of the chain before freeing it. The
buckets are nulled out rather than deleted, as all callers are iterating over
the hash at that point; they clean it right afterwards.
(cherry picked from commit 9d6fd4f)
ndossche
approved these changes
Oct 24, 2026
ndossche
left a comment
Member
There was a problem hiding this comment.
I didn't check the actual code change itself, only whether the backport is done correctly, and it is.
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
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.
Backports 207dd1d (GH-22480), 250f138 and 9d6fd4f from master, as suggested in GH-24549. Without them watch_006 is xfail on PHP-8.5 and on 32-bit PHP-8.4, and it passes on some 8.5 builds, which is the XPASS warning in the report.
Fixes GH-24549