From 11d6bea98add8ecf356e9cc2c36389c9046310cc Mon Sep 17 00:00:00 2001 From: Ilija Tovilo Date: Thu, 20 Jul 2023 13:28:02 +0200 Subject: [PATCH] Fix leaking definitions on FFI::cdef()->new() Previously, FFI_G(symbols) and FFI_G(tags) were never cleaned up when calling new on an existing object. However, if cdef() is called without parameters these globals are NULL and might be created when new() creates new definitions. These would then be discarded without freeing them. Closes GH-11751 --- NEWS | 3 +- ext/ffi/ffi.c | 112 +++++++++++++++++------------------- ext/ffi/tests/cdef_new.phpt | 14 +++++ 3 files changed, 69 insertions(+), 60 deletions(-) create mode 100644 ext/ffi/tests/cdef_new.phpt diff --git a/NEWS b/NEWS index 0fe7b382d0d..a0e3b0ebee8 100644 --- a/NEWS +++ b/NEWS @@ -2,7 +2,8 @@ PHP NEWS ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||| ?? ??? ????, PHP 8.1.23 - +- FFI: + . Fix leaking definitions when using FFI::cdef()->new(...). (ilutov) 03 Aug 2023, PHP 8.1.22 diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c index ee5183ce9d4..efb4524c76b 100644 --- a/ext/ffi/ffi.c +++ b/ext/ffi/ffi.c @@ -3684,22 +3684,22 @@ ZEND_METHOD(FFI, new) /* {{{ */ FFI_G(symbols) = NULL; FFI_G(tags) = NULL; } + bool clean_symbols = FFI_G(symbols) == NULL; + bool clean_tags = FFI_G(tags) == NULL; FFI_G(default_type_attr) = 0; if (zend_ffi_parse_type(ZSTR_VAL(type_def), ZSTR_LEN(type_def), &dcl) != SUCCESS) { zend_ffi_type_dtor(dcl.type); - if (Z_TYPE(EX(This)) != IS_OBJECT) { - if (FFI_G(tags)) { - zend_hash_destroy(FFI_G(tags)); - efree(FFI_G(tags)); - FFI_G(tags) = NULL; - } - if (FFI_G(symbols)) { - zend_hash_destroy(FFI_G(symbols)); - efree(FFI_G(symbols)); - FFI_G(symbols) = NULL; - } + if (clean_tags && FFI_G(tags)) { + zend_hash_destroy(FFI_G(tags)); + efree(FFI_G(tags)); + FFI_G(tags) = NULL; + } + if (clean_symbols && FFI_G(symbols)) { + zend_hash_destroy(FFI_G(symbols)); + efree(FFI_G(symbols)); + FFI_G(symbols) = NULL; } return; } @@ -3709,15 +3709,13 @@ ZEND_METHOD(FFI, new) /* {{{ */ is_const = 1; } - if (Z_TYPE(EX(This)) != IS_OBJECT) { - if (FFI_G(tags)) { - zend_ffi_tags_cleanup(&dcl); - } - if (FFI_G(symbols)) { - zend_hash_destroy(FFI_G(symbols)); - efree(FFI_G(symbols)); - FFI_G(symbols) = NULL; - } + if (clean_tags && FFI_G(tags)) { + zend_ffi_tags_cleanup(&dcl); + } + if (clean_symbols && FFI_G(symbols)) { + zend_hash_destroy(FFI_G(symbols)); + efree(FFI_G(symbols)); + FFI_G(symbols) = NULL; } FFI_G(symbols) = NULL; FFI_G(tags) = NULL; @@ -3828,22 +3826,22 @@ ZEND_METHOD(FFI, cast) /* {{{ */ FFI_G(symbols) = NULL; FFI_G(tags) = NULL; } + bool clean_symbols = FFI_G(symbols) == NULL; + bool clean_tags = FFI_G(tags) == NULL; FFI_G(default_type_attr) = 0; if (zend_ffi_parse_type(ZSTR_VAL(type_def), ZSTR_LEN(type_def), &dcl) != SUCCESS) { zend_ffi_type_dtor(dcl.type); - if (Z_TYPE(EX(This)) != IS_OBJECT) { - if (FFI_G(tags)) { - zend_hash_destroy(FFI_G(tags)); - efree(FFI_G(tags)); - FFI_G(tags) = NULL; - } - if (FFI_G(symbols)) { - zend_hash_destroy(FFI_G(symbols)); - efree(FFI_G(symbols)); - FFI_G(symbols) = NULL; - } + if (clean_tags && FFI_G(tags)) { + zend_hash_destroy(FFI_G(tags)); + efree(FFI_G(tags)); + FFI_G(tags) = NULL; + } + if (clean_symbols && FFI_G(symbols)) { + zend_hash_destroy(FFI_G(symbols)); + efree(FFI_G(symbols)); + FFI_G(symbols) = NULL; } return; } @@ -3853,15 +3851,13 @@ ZEND_METHOD(FFI, cast) /* {{{ */ is_const = 1; } - if (Z_TYPE(EX(This)) != IS_OBJECT) { - if (FFI_G(tags)) { - zend_ffi_tags_cleanup(&dcl); - } - if (FFI_G(symbols)) { - zend_hash_destroy(FFI_G(symbols)); - efree(FFI_G(symbols)); - FFI_G(symbols) = NULL; - } + if (clean_tags && FFI_G(tags)) { + zend_ffi_tags_cleanup(&dcl); + } + if (clean_symbols && FFI_G(symbols)) { + zend_hash_destroy(FFI_G(symbols)); + efree(FFI_G(symbols)); + FFI_G(symbols) = NULL; } FFI_G(symbols) = NULL; FFI_G(tags) = NULL; @@ -3994,35 +3990,33 @@ ZEND_METHOD(FFI, type) /* {{{ */ FFI_G(symbols) = NULL; FFI_G(tags) = NULL; } + bool clean_symbols = FFI_G(symbols) == NULL; + bool clean_tags = FFI_G(tags) == NULL; FFI_G(default_type_attr) = 0; if (zend_ffi_parse_type(ZSTR_VAL(type_def), ZSTR_LEN(type_def), &dcl) != SUCCESS) { zend_ffi_type_dtor(dcl.type); - if (Z_TYPE(EX(This)) != IS_OBJECT) { - if (FFI_G(tags)) { - zend_hash_destroy(FFI_G(tags)); - efree(FFI_G(tags)); - FFI_G(tags) = NULL; - } - if (FFI_G(symbols)) { - zend_hash_destroy(FFI_G(symbols)); - efree(FFI_G(symbols)); - FFI_G(symbols) = NULL; - } + if (clean_tags && FFI_G(tags)) { + zend_hash_destroy(FFI_G(tags)); + efree(FFI_G(tags)); + FFI_G(tags) = NULL; } - return; - } - - if (Z_TYPE(EX(This)) != IS_OBJECT) { - if (FFI_G(tags)) { - zend_ffi_tags_cleanup(&dcl); - } - if (FFI_G(symbols)) { + if (clean_symbols && FFI_G(symbols)) { zend_hash_destroy(FFI_G(symbols)); efree(FFI_G(symbols)); FFI_G(symbols) = NULL; } + return; + } + + if (clean_tags && FFI_G(tags)) { + zend_ffi_tags_cleanup(&dcl); + } + if (clean_symbols && FFI_G(symbols)) { + zend_hash_destroy(FFI_G(symbols)); + efree(FFI_G(symbols)); + FFI_G(symbols) = NULL; } FFI_G(symbols) = NULL; FFI_G(tags) = NULL; diff --git a/ext/ffi/tests/cdef_new.phpt b/ext/ffi/tests/cdef_new.phpt new file mode 100644 index 00000000000..ca3cba5d459 --- /dev/null +++ b/ext/ffi/tests/cdef_new.phpt @@ -0,0 +1,14 @@ +--TEST-- +Definitions should not leak when using FFI::cdef()->new(...) +--EXTENSIONS-- +ffi +--FILE-- +new('struct Example { uint32_t x; }'); +var_dump($struct); +?> +--EXPECT-- +object(FFI\CData:struct Example)#2 (1) { + ["x"]=> + int(0) +}