[php-src] master: Fix double free of phpdbg watch element chains

Author: Arnaud Le Blanc (arnaud-lb)
Date: 2026-08-27T13:25:07+02:00

Commit: https://github.com/php/php-src/commit/9d6fd4fd98aa9e7662f7b8082ce328b20c6be7b6
Raw diff: https://github.com/php/php-src/commit/9d6fd4fd98aa9e7662f7b8082ce328b20c6be7b6.diff

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.

Changed paths:
  M  sapi/phpdbg/phpdbg_watch.c


Diff:

diff --git a/sapi/phpdbg/phpdbg_watch.c b/sapi/phpdbg/phpdbg_watch.c
index 1e67d2c57670..a2e39e15a6ca 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));