From c6ff5fcf62a1ebbcb59443be27bc494bb3ec9d35 Mon Sep 17 00:00:00 2001 From: Ilia Alshanetsky Date: Sun, 14 Jun 2026 12:01:11 -0400 Subject: [PATCH] Fix SplObjectStorage getHash() guard leaking across a bailout The getHash() recursion guard increments a request-persistent counter around the userland getHash() call but decrements it only on the normal return path. A bailout inside an overridden getHash() (out-of-memory, timeout, or any fatal) skips the decrement, and the counter is never reset per request, so on a persistent SAPI every later request on the same worker wrongly throws "Modification of SplObjectStorage during getHash() is prohibited". Reset the counter in the SPL request init so each request starts at zero regardless of how the previous one exited. --- ext/spl/php_spl.c | 1 + ext/spl/spl_observer.c | 5 +++ ext/spl/spl_observer.h | 2 + .../spl_object_storage_gethash_bailout.phpt | 44 +++++++++++++++++++ 4 files changed, 52 insertions(+) create mode 100644 ext/spl/tests/spl_object_storage_gethash_bailout.phpt diff --git a/ext/spl/php_spl.c b/ext/spl/php_spl.c index 2477b371ea4e..0610e79196f9 100644 --- a/ext/spl/php_spl.c +++ b/ext/spl/php_spl.c @@ -563,6 +563,7 @@ PHP_MINIT_FUNCTION(spl) PHP_RINIT_FUNCTION(spl) /* {{{ */ { spl_autoload_extensions = NULL; + spl_object_storage_reset_get_hash_depth(); return SUCCESS; } /* }}} */ diff --git a/ext/spl/spl_observer.c b/ext/spl/spl_observer.c index f897ab1350cc..613bf5384dcb 100644 --- a/ext/spl/spl_observer.c +++ b/ext/spl/spl_observer.c @@ -47,6 +47,11 @@ static zend_object_handlers spl_handler_MultipleIterator; ZEND_TLS uint32_t spl_object_storage_get_hash_depth; +void spl_object_storage_reset_get_hash_depth(void) +{ + spl_object_storage_get_hash_depth = 0; +} + typedef struct _spl_SplObjectStorage { /* {{{ */ HashTable storage; zend_long index; diff --git a/ext/spl/spl_observer.h b/ext/spl/spl_observer.h index 08d3126d9c8b..bbb3ed656ccb 100644 --- a/ext/spl/spl_observer.h +++ b/ext/spl/spl_observer.h @@ -31,4 +31,6 @@ extern PHPAPI zend_class_entry *spl_ce_MultipleIterator; PHP_MINIT_FUNCTION(spl_observer); +void spl_object_storage_reset_get_hash_depth(void); + #endif /* SPL_OBSERVER_H */ diff --git a/ext/spl/tests/spl_object_storage_gethash_bailout.phpt b/ext/spl/tests/spl_object_storage_gethash_bailout.phpt new file mode 100644 index 000000000000..a6327bd62095 --- /dev/null +++ b/ext/spl/tests/spl_object_storage_gethash_bailout.phpt @@ -0,0 +1,44 @@ +--TEST-- +SplObjectStorage getHash() depth counter is reset after a bailout in a user getHash() +--SKIPIF-- + +--INI-- +allow_url_fopen=1 +--FILE-- +offsetSet(new stdClass()); + echo "poison"; +} else { + $s = new SplObjectStorage(); + $s->offsetSet(new stdClass()); + echo "check-ok count=", count($s); +} +PHP; + +php_cli_server_start($code, 'router.php'); + +$base = 'http://' . PHP_CLI_SERVER_ADDRESS; +// Request 1 bails out (OOM) inside the overridden getHash() mid-offsetSet. +@file_get_contents($base . '/poison'); +// A later request on the same worker must not be poisoned by a stuck counter. +echo @file_get_contents($base . '/check'), "\n"; +echo @file_get_contents($base . '/check'), "\n"; +?> +--EXPECT-- +check-ok count=1 +check-ok count=1