Commit 9d6fd4fd98a for php.net
commit 9d6fd4fd98aa9e7662f7b8082ce328b20c6be7b6
Author: Arnaud Le Blanc <arnaud.lb@gmail.com>
Date: Wed Aug 26 18:25:39 2026 +0200
Fix double free of phpdbg watch element chains
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.
diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c
index 1e67d2c5767..a2e39e15a6c 100644
--- a/sapi/phpdbg/phpdbg_watch.c
+++ b/sapi/phpdbg/phpdbg_watch.c
@@ -847,8 +847,36 @@ bool phpdbg_try_re_adding_watch_element(zval *parent, phpdbg_watch_element *elem
return true;
}
+/* watch_recreation is keyed by element->str, so a parent and its child are stored
+ * under distinct keys and the deduplication in phpdbg_queue_element_for_recreation()
+ * cannot notice that they belong to the same chain. As phpdbg_free_watch_element_tree()
+ * frees the whole chain, any other entry referencing a member of it must be dropped
+ * first, or the chain gets freed twice. The buckets are only nulled out as the hash is
+ * being iterated over by the callers; they clean it right after. */
+static void phpdbg_forget_queued_watch_element(phpdbg_watch_element *element) {
+ zval *zv = zend_hash_find(&PHPDBG_G(watch_recreation), element->str);
+ if (zv && Z_PTR_P(zv) == element) {
+ Z_PTR_P(zv) = NULL;
+ }
+}
+
+static void phpdbg_dequeue_watch_element_tree(phpdbg_watch_element *element) {
+ phpdbg_watch_element *cur;
+
+ for (cur = element->parent; cur; cur = cur->parent) {
+ phpdbg_forget_queued_watch_element(cur);
+ }
+ for (cur = element->child; cur; cur = cur->child) {
+ phpdbg_forget_queued_watch_element(cur);
+ }
+ phpdbg_forget_queued_watch_element(element);
+}
+
void phpdbg_automatic_dequeue_free(phpdbg_watch_element *element) {
phpdbg_watch_element *child = element;
+
+ phpdbg_dequeue_watch_element_tree(element);
+
while (child->child && !(child->flags & PHPDBG_WATCH_RECURSIVE_ROOT)) {
child = child->child;
}
@@ -863,6 +891,10 @@ void phpdbg_dequeue_elements_for_recreation(void) {
phpdbg_watch_element *element;
ZEND_HASH_MAP_FOREACH_PTR(&PHPDBG_G(watch_recreation), element) {
+ if (!element) {
+ /* freed along with an already dequeued element of the same chain */
+ continue;
+ }
ZEND_ASSERT(element->flags & (PHPDBG_WATCH_IMPLICIT | PHPDBG_WATCH_RECURSIVE_ROOT | PHPDBG_WATCH_SIMPLE));
if (element->parent || zend_hash_index_find(&PHPDBG_G(watch_free), (zend_ulong)(uintptr_t) element->parent_container)) {
zval _zv, *zv = &_zv;
@@ -1641,7 +1673,9 @@ void phpdbg_destroy_watchpoints(void) {
/* unconditionally free all remaining elements to avoid memory leaks */
ZEND_HASH_MAP_FOREACH_PTR(&PHPDBG_G(watch_recreation), element) {
- phpdbg_automatic_dequeue_free(element);
+ if (element) {
+ phpdbg_automatic_dequeue_free(element);
+ }
} ZEND_HASH_FOREACH_END();
/* upon fatal errors etc. (i.e. CG(unclean_shutdown) == 1), some watchpoints may still be active. Ensure memory is not watched anymore for next run. Do not care about memory freeing here, shutdown is unclean and near anyway. */
@@ -1669,7 +1703,9 @@ void phpdbg_release_watch_elements(void) {
uint32_t guard;
ZEND_HASH_MAP_FOREACH_PTR(&PHPDBG_G(watch_recreation), element) {
- phpdbg_automatic_dequeue_free(element);
+ if (element) {
+ phpdbg_automatic_dequeue_free(element);
+ }
} ZEND_HASH_FOREACH_END();
zend_hash_clean(&PHPDBG_G(watch_recreation));