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.

diff --git a/Zend/zend_generators.c b/Zend/zend_generators.c
index 1162c7653f05a1e738bd9fb7b3aa2461686a468c..d3fa86b4144a8e8fa69103639311f5a47a0f996d 100644 (file)
--- 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 f757f282bcd703965e5105f04ee6d637e7c5aca2..3389b646081624f9b1b9a5273ae29904ce0038fc 100644 (file)
--- 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);