diff --git a/ext/libxml/libxml.c b/ext/libxml/libxml.c index d09ec299ff3c..285c8ecb9f38 100644 --- a/ext/libxml/libxml.c +++ b/ext/libxml/libxml.c @@ -1041,7 +1041,10 @@ static PHP_RINIT_FUNCTION(libxml) static PHP_RSHUTDOWN_FUNCTION(libxml) { if (ZEND_FCC_INITIALIZED(LIBXML(entity_loader_callback))) { - zend_fcc_dtor(&LIBXML(entity_loader_callback)); + /* Detach before destruction, as destroying the callback may bail out. */ + zend_fcall_info_cache old_fcc = LIBXML(entity_loader_callback); + LIBXML(entity_loader_callback) = empty_fcall_info_cache; + zend_fcc_dtor(&old_fcc); } return SUCCESS; @@ -1261,13 +1264,18 @@ PHP_FUNCTION(libxml_set_external_entity_loader) Z_PARAM_FUNC_NO_TRAMPOLINE_FREE_OR_NULL(fci, fcc) ZEND_PARSE_PARAMETERS_END(); - /* Unset old callback if it's defined */ - if (ZEND_FCC_INITIALIZED(LIBXML(entity_loader_callback))) { - zend_fcc_dtor(&LIBXML(entity_loader_callback)); - } + /* Detach the old callback before destruction, which may re-enter this function. */ + zend_fcall_info_cache old_fcc = LIBXML(entity_loader_callback); + LIBXML(entity_loader_callback) = empty_fcall_info_cache; + if (ZEND_FCI_INITIALIZED(fci)) { /* argument not null */ zend_fcc_dup(&LIBXML(entity_loader_callback), &fcc); } + + /* Destroy only after installing the replacement so re-entrant changes are preserved. */ + if (ZEND_FCC_INITIALIZED(old_fcc)) { + zend_fcc_dtor(&old_fcc); + } RETURN_TRUE; } /* }}} */ diff --git a/ext/libxml/tests/libxml_entity_loader_bailout.phpt b/ext/libxml/tests/libxml_entity_loader_bailout.phpt new file mode 100644 index 000000000000..2b6ab483bb7b --- /dev/null +++ b/ext/libxml/tests/libxml_entity_loader_bailout.phpt @@ -0,0 +1,38 @@ +--TEST-- +GH-24763: libxml external entity loader is detached before callback destruction bailout +--EXTENSIONS-- +libxml +zend_test +--FILE-- + +--EXPECTF-- +Fatal error: Bailout in %s on line %d +NULL diff --git a/ext/libxml/tests/libxml_entity_loader_reentrant_replace.phpt b/ext/libxml/tests/libxml_entity_loader_reentrant_replace.phpt new file mode 100644 index 000000000000..e0879a5fb65a --- /dev/null +++ b/ext/libxml/tests/libxml_entity_loader_reentrant_replace.phpt @@ -0,0 +1,33 @@ +--TEST-- +libxml external entity loader replacement is safe against destructor reentrancy +--EXTENSIONS-- +libxml +--FILE-- + null); + } +} + +$d = new D(); + +$oldLoader = function () use ($d) { +}; + +unset($d); + +libxml_set_external_entity_loader($oldLoader); +unset($oldLoader); + +libxml_set_external_entity_loader(fn() => 1); + +$loader = libxml_get_external_entity_loader(); +var_dump($loader()); + +?> +--EXPECT-- +NULL