Fix GH-8433: Assigning function pointers to structs in FFI leaks memory

php · Apr 24, 2022 · 2397e76 · 2397e76
1 parent f5d9e7c
commit 2397e76
Show file tree

Hide file tree

Showing 3 changed files with 54 additions and 3 deletions.
diff --git a/NEWS b/NEWS
@@ -10,6 +10,10 @@ PHP                                                                        NEWS
   . Fixed bug GH-7979 (DatePeriod iterator advances when checking if valid).
     (Derick, Cody Mann)
 
+- FFI:
+  . Fixed bug GH-8433 (Assigning function pointers to structs in FFI leaks).
+    (Bob)
+
 - FPM:
   . Fixed bug #76003 (FPM /status reports wrong number of active processe).
     (Jakub Zelenka)

diff --git a/ext/ffi/ffi.c b/ext/ffi/ffi.c
@@ -864,6 +864,14 @@ static void zend_ffi_callback_hash_dtor(zval *zv) /* {{{ */
 	if (callback_data->fcc.function_handler->common.fn_flags & ZEND_ACC_CLOSURE) {
 		OBJ_RELEASE(ZEND_CLOSURE_OBJECT(callback_data->fcc.function_handler));
 	}
+	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]);
+		}
+	}
+	if (callback_data->ret_type->type == FFI_TYPE_STRUCT) {
+		efree(callback_data->ret_type);
+	}
 	efree(callback_data);
 }
 /* }}} */
@@ -917,6 +925,8 @@ static void zend_ffi_callback_trampoline(ffi_cif* cif, void* ret, void** args, v
 	if (ret_type->kind != ZEND_FFI_TYPE_VOID) {
 		zend_ffi_zval_to_cdata(ret, ret_type, &retval);
 	}
+
+	zval_ptr_dtor(&retval);
 }
 /* }}} */
 
@@ -967,6 +977,11 @@ static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value) /* {{{ *
 			callback_data->arg_types[n] = zend_ffi_get_type(arg_type);
 			if (!callback_data->arg_types[n]) {
 				zend_ffi_pass_unsupported(arg_type);
+				for (int i = 0; i < n; ++i) {
+					if (callback_data->arg_types[i]->type == FFI_TYPE_STRUCT) {
+						efree(callback_data->arg_types[i]);
+					}
+				}
 				efree(callback_data);
 				ffi_closure_free(callback);
 				return NULL;
@@ -977,20 +992,32 @@ static void *zend_ffi_create_callback(zend_ffi_type *type, zval *value) /* {{{ *
 	callback_data->ret_type = zend_ffi_get_type(ZEND_FFI_TYPE(type->func.ret_type));
 	if (!callback_data->ret_type) {
 		zend_ffi_return_unsupported(type->func.ret_type);
+		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]);
+			}
+		}
 		efree(callback_data);
 		ffi_closure_free(callback);
 		return NULL;
 	}
 
 	if (ffi_prep_cif(&callback_data->cif, type->func.abi, callback_data->arg_count, callback_data->ret_type, callback_data->arg_types) != FFI_OK) {
 		zend_throw_error(zend_ffi_exception_ce, "Cannot prepare callback CIF");
-		efree(callback_data);
-		ffi_closure_free(callback);
-		return NULL;
+		goto free_on_failure;
 	}
 
 	if (ffi_prep_closure_loc(callback, &callback_data->cif, zend_ffi_callback_trampoline, callback_data, code) != FFI_OK) {
 		zend_throw_error(zend_ffi_exception_ce, "Cannot prepare callback");
+free_on_failure: ;
+		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]);
+			}
+		}
+		if (callback_data->ret_type->type == FFI_TYPE_STRUCT) {
+			efree(callback_data->ret_type);
+		}
 		efree(callback_data);
 		ffi_closure_free(callback);
 		return NULL;

diff --git a/ext/ffi/tests/gh8433.phpt b/ext/ffi/tests/gh8433.phpt
@@ -0,0 +1,20 @@
+--TEST--
+GH-8433 (Assigning function pointers to structs in FFI leaks memory)
+--FILE--
+<?php
+
+$ffi = FFI::cdef("typedef struct { int a; } bar;");
+$x = $ffi->new("bar(*)(void)");
+FFI::addr($x)[0] = function() use ($ffi) {
+    $bar = $ffi->new("bar");
+    $bar->a = 2;
+    return $bar;
+};
+var_dump($x());
+
+?>
+--EXPECTF--
+object(FFI\CData:struct <anonymous>)#%d (1) {
+  ["a"]=>
+  int(2)
+}