From 4dc0662eca4d8bc79cb6f6fa0c62e1600efef78a Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 1 Mar 2019 14:32:11 +0100 Subject: [PATCH] Check for NULL GC type in objects_store_del This might happen if OBJ_RELEASE is used on an object that was already released by GC. Specific cases of this issue were previously fixed in ffaee27478a9cb338e40edeb5acf233f9cb67111 and 72104d2b6ecbbabd18de15f10739be5ce3dc9ce0, however the issue still affects 3rd-party extensions using OBJ_RELEASE. The whole GC type NULL + OBJ_IS_VALID + IS_FREE_CALLED system seems overly complicated and can probably be simplified in 7.4. --- Zend/zend_generators.c | 6 ++---- Zend/zend_objects_API.c | 13 +++++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c index 1162c7653f..d3fa86b414 100644 --- a/Zend/zend_generators.c +++ b/Zend/zend_generators.c @@ -123,8 +123,7 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished /* always free the CV's, in the symtable are only not-free'd IS_INDIRECT's */ zend_free_compiled_variables(execute_data); - if ((EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) && - EXPECTED(GC_TYPE(Z_OBJ(execute_data->This)) == IS_OBJECT)) { + if (EX_CALL_INFO() & ZEND_CALL_RELEASE_THIS) { OBJ_RELEASE(Z_OBJ(execute_data->This)); } @@ -144,8 +143,7 @@ ZEND_API void zend_generator_close(zend_generator *generator, zend_bool finished } /* Free closure object */ - if ((EX_CALL_INFO() & ZEND_CALL_CLOSURE) && - EXPECTED(GC_TYPE(ZEND_CLOSURE_OBJECT(EX(func))) == IS_OBJECT)) { + if (EX_CALL_INFO() & ZEND_CALL_CLOSURE) { OBJ_RELEASE(ZEND_CLOSURE_OBJECT(EX(func))); } diff --git a/Zend/zend_objects_API.c b/Zend/zend_objects_API.c index f757f282bc..3389b64608 100644 --- a/Zend/zend_objects_API.c +++ b/Zend/zend_objects_API.c @@ -152,14 +152,17 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_put(zend_object *object) ZEND_API void ZEND_FASTCALL zend_objects_store_del(zend_object *object) /* {{{ */ { + ZEND_ASSERT(GC_REFCOUNT(object) == 0); + + /* GC might have released this object already. */ + if (UNEXPECTED(GC_TYPE(object) == IS_NULL)) { + return; + } + /* Make sure we hold a reference count during the destructor call otherwise, when the destructor ends the storage might be freed when the refcount reaches 0 a second time */ - ZEND_ASSERT(EG(objects_store).object_buckets != NULL); - ZEND_ASSERT(IS_OBJ_VALID(EG(objects_store).object_buckets[object->handle])); - ZEND_ASSERT(GC_REFCOUNT(object) == 0); - if (!(OBJ_FLAGS(object) & IS_OBJ_DESTRUCTOR_CALLED)) { GC_ADD_FLAGS(object, IS_OBJ_DESTRUCTOR_CALLED); @@ -176,6 +179,8 @@ ZEND_API void ZEND_FASTCALL zend_objects_store_del(zend_object *object) /* {{{ * uint32_t handle = object->handle; void *ptr; + ZEND_ASSERT(EG(objects_store).object_buckets != NULL); + ZEND_ASSERT(IS_OBJ_VALID(EG(objects_store).object_buckets[object->handle])); EG(objects_store).object_buckets[handle] = SET_OBJ_INVALID(object); if (!(OBJ_FLAGS(object) & IS_OBJ_FREE_CALLED)) { GC_ADD_FLAGS(object, IS_OBJ_FREE_CALLED); -- 2.50.1