Skip to content

Commit

Permalink
fix double free call, which caused memory-corruption
Browse files Browse the repository at this point in the history
  • Loading branch information
EmielBruijntjes committed Oct 13, 2024
1 parent 87b079a commit 2c0a231
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 4 deletions.
7 changes: 5 additions & 2 deletions zend/classimpl.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
* Implementation file for the ClassImpl class
*
* @author Emiel Bruijntjes <[email protected]>
* @copyright 2014 - 2019 Copernica BV
* @copyright 2014 - 2024 Copernica BV
*/
#include "includes.h"
#include <cstring>
Expand Down Expand Up @@ -207,7 +207,10 @@ zend_function *ClassImpl::getMethod(zend_object **object, zend_string *method, c
// had an implementation here that used a static variable, and that worked too,
// but we'll follow thread safe implementation of the Zend engine here, although
// it is strange to allocate and free memory in one and the same method call (free()
// call happens in call_method())
// call happens in call_method()) (2024-10-13 extra info: the method_exists()
// function and our own Value::isCallable() method expect this to be emalloc()-
// allocated buffer, because they both call zend_free_trampoline() (which is
// effectively an efree() call) on the returned function-structure)
auto *data = (CallData *)emalloc(sizeof(CallData));
auto *function = &data->func;

Expand Down
7 changes: 5 additions & 2 deletions zend/value.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -891,8 +891,11 @@ bool Value::isCallable(const char *name)
bool result = func->common.scope == zend_ce_closure && zend_string_equals_cstr(methodname.value(), ZEND_INVOKE_FUNC_NAME, ::strlen(ZEND_INVOKE_FUNC_NAME));
#endif

// free resources (still don't get this code, copied from zend_builtin_functions.c)
zend_string_release(func->common.function_name);
// in method_exists(), there is also a zend_string_release() call here, but I dont think we
// need it here, because the methodname is already cleanup by the destructor of the LowerCase class
//zend_string_release(func->common.function_name);

// free resources, just like method_exists() does
zend_free_trampoline(func);

// done
Expand Down

0 comments on commit 2c0a231

Please sign in to comment.