Commit ed60b9edebd for php
commit ed60b9edebd884b4cb82e1135b81fc50ce43aea6
Author: Ilia Alshanetsky <ilia@ilia.ws>
Date: Sat Sep 26 18:15:05 2026 -0400
ext/ffi: Hold a reference to the callable of an FFI callback
zend_ffi_create_callback() stored the callable's fcall info cache without
owning it and passed it to zend_call_function(), which clears
function_handler before running a __call() trampoline. Calling such a
callback twice failed, destroying it read a NULL handler, an array
callable's object could be freed while the callback still used it, and a
failed creation left the trampoline in EG(trampoline). Take ownership with
zend_fcc_addref(), call through zend_call_known_fcc(), release with
zend_fcc_dtor(), and release the cache when creation fails.
Closes GH-23933
diff --git a/NEWS b/NEWS
index fb25e3a0b80..bde051dee66 100644
--- a/NEWS
+++ b/NEWS
@@ -32,6 +32,10 @@ PHP NEWS
. Fixed bug GH-23897 (php:function() assertion failure after a failed
registerPHPFunctions()). (David Carlier)
+- FFI:
+ . Fixed crashes with FFI callbacks created from __call() trampolines
+ and array callables whose object is released. (Ilia Alshanetsky)
+
- FTP:
. Fixed bug GH-23619 (cryptic error on servers that don't support TLS
session resumption on data connection). (ndossche)
diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
index 6fb00a330d8..79783441c1d 100644
--- a/ext/ffi/ffi.c
+++ b/ext/ffi/ffi.c
@@ -923,9 +923,7 @@ static void zend_ffi_callback_hash_dtor(zval *zv) /* {{{ */
zend_ffi_callback_data *callback_data = Z_PTR_P(zv);
ffi_closure_free(callback_data->callback);
- if (callback_data->fcc.function_handler->common.fn_flags & ZEND_ACC_CLOSURE) {
- OBJ_RELEASE(ZEND_CLOSURE_OBJECT(callback_data->fcc.function_handler));
- }
+ zend_fcc_dtor(&callback_data->fcc);
for (int i = 0; i < callback_data->arg_count; ++i) {
if (callback_data->arg_types[i]->type == FFI_TYPE_STRUCT) {
efree(callback_data->arg_types[i]);
@@ -941,18 +939,12 @@ static void zend_ffi_callback_hash_dtor(zval *zv) /* {{{ */
static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, void* data) /* {{{ */
{
zend_ffi_callback_data *callback_data = (zend_ffi_callback_data*)data;
- zend_fcall_info fci;
+ zval *params;
zend_ffi_type *ret_type;
zval retval;
ALLOCA_FLAG(use_heap)
- fci.size = sizeof(zend_fcall_info);
- ZVAL_UNDEF(&fci.function_name);
- fci.retval = &retval;
- fci.params = do_alloca(sizeof(zval) *callback_data->arg_count, use_heap);
- fci.object = NULL;
- fci.param_count = callback_data->arg_count;
- fci.named_params = NULL;
+ params = do_alloca(sizeof(zval) *callback_data->arg_count, use_heap);
if (callback_data->type->func.args) {
int n = 0;
@@ -960,24 +952,21 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
ZEND_HASH_PACKED_FOREACH_PTR(callback_data->type->func.args, arg_type) {
arg_type = ZEND_FFI_TYPE(arg_type);
- zend_ffi_cdata_to_zval(NULL, args[n], arg_type, BP_VAR_R, &fci.params[n], (zend_ffi_flags)(arg_type->attr & ZEND_FFI_ATTR_CONST), 0, 0);
+ zend_ffi_cdata_to_zval(NULL, args[n], arg_type, BP_VAR_R, ¶ms[n], (zend_ffi_flags)(arg_type->attr & ZEND_FFI_ATTR_CONST), 0, 0);
n++;
} ZEND_HASH_FOREACH_END();
}
- ZVAL_UNDEF(&retval);
- if (zend_call_function(&fci, &callback_data->fcc) != SUCCESS) {
- zend_throw_error(zend_ffi_exception_ce, "Cannot call callback");
- }
+ zend_call_known_fcc(&callback_data->fcc, &retval, callback_data->arg_count, params, NULL);
if (callback_data->arg_count) {
int n = 0;
for (n = 0; n < callback_data->arg_count; n++) {
- zval_ptr_dtor(&fci.params[n]);
+ zval_ptr_dtor(¶ms[n]);
}
}
- free_alloca(fci.params, use_heap);
+ free_alloca(params, use_heap);
if (EG(exception)) {
zend_error_noreturn(E_ERROR, "Throwing from FFI callbacks is not allowed");
@@ -1035,12 +1024,14 @@ static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value) /* {{{ *
arg_count = type->func.args ? zend_hash_num_elements(type->func.args) : 0;
if (arg_count < fcc.function_handler->common.required_num_args) {
zend_throw_error(zend_ffi_exception_ce, "Attempt to assign an invalid callback, insufficient number of arguments");
+ zend_release_fcall_info_cache(&fcc);
return NULL;
}
callback = ffi_closure_alloc(sizeof(ffi_closure), &code);
if (!callback) {
zend_throw_error(zend_ffi_exception_ce, "Cannot allocate callback");
+ zend_release_fcall_info_cache(&fcc);
return NULL;
}
@@ -1067,6 +1058,7 @@ static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value) /* {{{ *
}
efree(callback_data);
ffi_closure_free(callback);
+ zend_release_fcall_info_cache(&fcc);
return NULL;
}
n++;
@@ -1082,6 +1074,7 @@ static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value) /* {{{ *
}
efree(callback_data);
ffi_closure_free(callback);
+ zend_release_fcall_info_cache(&fcc);
return NULL;
}
@@ -1103,6 +1096,7 @@ free_on_failure: ;
}
efree(callback_data);
ffi_closure_free(callback);
+ zend_release_fcall_info_cache(&fcc);
return NULL;
}
@@ -1112,9 +1106,7 @@ free_on_failure: ;
}
zend_hash_next_index_insert_ptr(FFI_G(callbacks), callback_data);
- if (fcc.function_handler->common.fn_flags & ZEND_ACC_CLOSURE) {
- GC_ADDREF(ZEND_CLOSURE_OBJECT(fcc.function_handler));
- }
+ zend_fcc_addref(&callback_data->fcc);
return code;
}
diff --git a/ext/ffi/tests/callback_create_failure.phpt b/ext/ffi/tests/callback_create_failure.phpt
new file mode 100644
index 00000000000..2809d9404c2
--- /dev/null
+++ b/ext/ffi/tests/callback_create_failure.phpt
@@ -0,0 +1,27 @@
+--TEST--
+FFI callback creation failure releases a __call trampoline
+--EXTENSIONS--
+ffi
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+$ffi = FFI::cdef("struct E {}; typedef int (*cb_t)(struct E); struct S { cb_t f; };");
+
+class Callback {
+ public function __call(string $name, array $arguments): int {
+ return 0;
+ }
+}
+
+$s = $ffi->new("struct S");
+try {
+ $s->f = [new Callback(), 'compare'];
+} catch (FFI\Exception $e) {
+ echo $e::class, ": ", $e->getMessage(), PHP_EOL;
+}
+echo "Done\n";
+?>
+--EXPECT--
+FFI\Exception: Cannot prepare callback CIF
+Done
diff --git a/ext/ffi/tests/callback_dtor_call.phpt b/ext/ffi/tests/callback_dtor_call.phpt
new file mode 100644
index 00000000000..40d72227d65
--- /dev/null
+++ b/ext/ffi/tests/callback_dtor_call.phpt
@@ -0,0 +1,33 @@
+--TEST--
+FFI callback bound to a __call() trampoline
+--EXTENSIONS--
+ffi
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+$ffi = FFI::cdef("typedef int (*cb_t)(int); struct S { cb_t f; cb_t g; };");
+
+class Callback {
+ public function __call(string $name, array $arguments): int {
+ echo $name, "(", $arguments[0], ")\n";
+
+ return $arguments[0] * 2;
+ }
+}
+
+$callback = new Callback();
+$s = $ffi->new("struct S");
+$s->g = [$callback, 'unused'];
+$s->f = [$callback, 'double'];
+var_dump(($s->f)(1));
+var_dump(($s->f)(2));
+var_dump(($s->f)(3));
+?>
+--EXPECT--
+double(1)
+int(2)
+double(2)
+int(4)
+double(3)
+int(6)
diff --git a/ext/ffi/tests/callback_object_lifetime.phpt b/ext/ffi/tests/callback_object_lifetime.phpt
new file mode 100644
index 00000000000..741dd5a1d74
--- /dev/null
+++ b/ext/ffi/tests/callback_object_lifetime.phpt
@@ -0,0 +1,31 @@
+--TEST--
+FFI callback keeps the object of an array callable alive
+--EXTENSIONS--
+ffi
+--INI--
+ffi.enable=1
+--FILE--
+<?php
+$ffi = FFI::cdef("typedef int (*cb_t)(int); struct S { cb_t f; };");
+
+class Callback {
+ public int $factor = 21;
+
+ public function multiply(int $x): int {
+ return $this->factor * $x;
+ }
+
+ public function __destruct() {
+ echo "Callback::__destruct\n";
+ }
+}
+
+$s = $ffi->new("struct S");
+$s->f = [new Callback(), 'multiply'];
+var_dump(($s->f)(2));
+echo "Done\n";
+?>
+--EXPECT--
+int(42)
+Done
+Callback::__destruct