From f095d2c91b7177fb0d9791cfc89bee66cf9d4c3e Mon Sep 17 00:00:00 2001 From: Bob Weinand Date: Mon, 7 Mar 2022 20:35:16 +0100 Subject: [PATCH 1/2] Fix freeing of internal attribute arguments --- NEWS | 3 +++ Zend/zend_attributes.c | 9 +++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/NEWS b/NEWS index d1537408024..604bacc7c75 100644 --- a/NEWS +++ b/NEWS @@ -2,6 +2,9 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? 2022, PHP 8.0.18 +- Core: + . Fixed freeing of internal attribute arguments. (Bob) + - Intl: . Fixed bug GH-8142 (Compilation error on cygwin). (David Carlier) diff --git a/Zend/zend_attributes.c b/Zend/zend_attributes.c index 58cd813f7df..45d1cbec46f 100644 --- a/Zend/zend_attributes.c +++ b/Zend/zend_attributes.c @@ -181,6 +181,7 @@ ZEND_API zend_bool zend_is_attribute_repeated(HashTable *attributes, zend_attrib static void attr_free(zval *v) { zend_attribute *attr = Z_PTR_P(v); + bool persistent = attr->flags & ZEND_ATTRIBUTE_PERSISTENT; zend_string_release(attr->name); zend_string_release(attr->lcname); @@ -189,10 +190,14 @@ static void attr_free(zval *v) if (attr->args[i].name) { zend_string_release(attr->args[i].name); } - zval_ptr_dtor(&attr->args[i].value); + if (persistent) { + zval_internal_ptr_dtor(&attr->args[i].value); + } else { + zval_ptr_dtor(&attr->args[i].value); + } } - pefree(attr, attr->flags & ZEND_ATTRIBUTE_PERSISTENT); + pefree(attr, persistent); } ZEND_API zend_attribute *zend_add_attribute(HashTable **attributes, zend_string *name, uint32_t argc, uint32_t flags, uint32_t offset, uint32_t lineno) From 0d7e10c1a9babcb2592fb5cd9284e779b45af2e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20D=C3=BCsterhus?= Date: Mon, 7 Mar 2022 16:32:46 +0100 Subject: [PATCH 2/2] Fix memory leak of function attribute hash table (#8070) ==109253== 280 (56 direct, 224 indirect) bytes in 1 blocks are definitely lost in loss record 4 of 4 ==109253== at 0x483B7F3: malloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so) ==109253== by 0x6D9FA2: __zend_malloc (zend_alloc.c:3068) ==109253== by 0x745138: zend_add_attribute (zend_attributes.c:226) ==109253== by 0x6680D1: zend_add_parameter_attribute (zend_attributes.h:102) ==109253== by 0x66B787: zm_startup_zend_test (test.c:478) ==109253== by 0x7224CD: zend_startup_module_ex (zend_API.c:2202) ==109253== by 0x72252C: zend_startup_module_zval (zend_API.c:2217) ==109253== by 0x734288: zend_hash_apply (zend_hash.c:2011) ==109253== by 0x722C30: zend_startup_modules (zend_API.c:2328) ==109253== by 0x67409B: php_module_startup (main.c:2256) ==109253== by 0x88EDDE: php_cli_startup (php_cli.c:409) ==109253== by 0x890F61: main (php_cli.c:1334) --- NEWS | 2 ++ Zend/zend.c | 16 ++++++++++++++++ Zend/zend_opcode.c | 19 +++++++++++++++---- 3 files changed, 33 insertions(+), 4 deletions(-) diff --git a/NEWS b/NEWS index 604bacc7c75..a4c779472d4 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ PHP NEWS - Core: . Fixed freeing of internal attribute arguments. (Bob) + . Fixed bug GH-8070 (memory leak of internal function attribute hash). + (Tim Düsterhus) - Intl: . Fixed bug GH-8142 (Compilation error on cygwin). (David Carlier) diff --git a/Zend/zend.c b/Zend/zend.c index cb245bb7704..e9f9388ae29 100644 --- a/Zend/zend.c +++ b/Zend/zend.c @@ -624,6 +624,22 @@ static void function_copy_ctor(zval *zv) /* {{{ */ } func->common.arg_info = new_arg_info + 1; } + if (old_func->common.attributes) { + zend_attribute *old_attr; + + func->common.attributes = NULL; + + ZEND_HASH_PACKED_FOREACH_PTR(old_func->common.attributes, old_attr) { + uint32_t i; + zend_attribute *attr; + + attr = zend_add_attribute(&func->common.attributes, old_attr->name, old_attr->argc, old_attr->flags, old_attr->offset, old_attr->lineno); + + for (i = 0 ; i < old_attr->argc; i++) { + ZVAL_DUP(&attr->args[i].value, &old_attr->args[i].value); + } + } ZEND_HASH_FOREACH_END(); + } } /* }}} */ diff --git a/Zend/zend_opcode.c b/Zend/zend_opcode.c index 620d42dfc6e..7ea2485e1c4 100644 --- a/Zend/zend_opcode.c +++ b/Zend/zend_opcode.c @@ -153,6 +153,11 @@ ZEND_API void zend_function_dtor(zval *zv) /* For methods this will be called explicitly. */ if (!function->common.scope) { zend_free_internal_arg_info(&function->internal_function); + + if (function->common.attributes) { + zend_hash_release(function->common.attributes); + function->common.attributes = NULL; + } } if (!(function->common.fn_flags & ZEND_ACC_ARENA_ALLOCATED)) { @@ -394,11 +399,17 @@ ZEND_API void destroy_zend_class(zval *zv) zend_hash_destroy(&ce->properties_info); zend_string_release_ex(ce->name, 1); - /* TODO: eliminate this loop for classes without functions with arg_info */ + /* TODO: eliminate this loop for classes without functions with arg_info / attributes */ ZEND_HASH_FOREACH_PTR(&ce->function_table, fn) { - if ((fn->common.fn_flags & (ZEND_ACC_HAS_RETURN_TYPE|ZEND_ACC_HAS_TYPE_HINTS)) && - fn->common.scope == ce) { - zend_free_internal_arg_info(&fn->internal_function); + if (fn->common.scope == ce) { + if (fn->common.fn_flags & (ZEND_ACC_HAS_RETURN_TYPE|ZEND_ACC_HAS_TYPE_HINTS)) { + zend_free_internal_arg_info(&fn->internal_function); + } + + if (fn->common.attributes) { + zend_hash_release(fn->common.attributes); + fn->common.attributes = NULL; + } } } ZEND_HASH_FOREACH_END();