Skip to content

Commit 00c0a9b

Browse files
committed
Merge branch 'PHP-8.5'
* PHP-8.5: ext/ffi: Fix resource leak in FFI::cdef() on symbol resolution failure.
2 parents be539d9 + b6b26f4 commit 00c0a9b

File tree

4 files changed

+56
-25
lines changed

4 files changed

+56
-25
lines changed

ext/ffi/ffi.c

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1253,10 +1253,8 @@ static zval *zend_ffi_cdata_read_field(zend_object *obj, zend_string *field_name
12531253
type = ZEND_FFI_TYPE(type->pointer.type);
12541254
}
12551255
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
1256-
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
1257-
zend_throw_error(zend_ffi_exception_ce, "Attempt to read field '%s' of non C struct/union", ZSTR_VAL(field_name));
1258-
return &EG(uninitialized_zval);
1259-
}
1256+
zend_throw_error(zend_ffi_exception_ce, "Attempt to read field '%s' of non C struct/union", ZSTR_VAL(field_name));
1257+
return &EG(uninitialized_zval);
12601258
}
12611259

12621260
field = zend_hash_find_ptr(&type->record.fields, field_name);
@@ -1331,10 +1329,8 @@ static zval *zend_ffi_cdata_write_field(zend_object *obj, zend_string *field_nam
13311329
type = ZEND_FFI_TYPE(type->pointer.type);
13321330
}
13331331
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
1334-
if (UNEXPECTED(type->kind != ZEND_FFI_TYPE_STRUCT)) {
1335-
zend_throw_error(zend_ffi_exception_ce, "Attempt to assign field '%s' of non C struct/union", ZSTR_VAL(field_name));
1336-
return value;
1337-
}
1332+
zend_throw_error(zend_ffi_exception_ce, "Attempt to assign field '%s' of non C struct/union", ZSTR_VAL(field_name));
1333+
return value;
13381334
}
13391335

13401336
field = zend_hash_find_ptr(&type->record.fields, field_name);
@@ -3107,17 +3103,7 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
31073103
FFI_G(default_type_attr) = ZEND_FFI_ATTR_STORED;
31083104

31093105
if (zend_ffi_parse_decl(ZSTR_VAL(code), ZSTR_LEN(code)) == FAILURE) {
3110-
if (FFI_G(symbols)) {
3111-
zend_hash_destroy(FFI_G(symbols));
3112-
efree(FFI_G(symbols));
3113-
FFI_G(symbols) = NULL;
3114-
}
3115-
if (FFI_G(tags)) {
3116-
zend_hash_destroy(FFI_G(tags));
3117-
efree(FFI_G(tags));
3118-
FFI_G(tags) = NULL;
3119-
}
3120-
RETURN_THROWS();
3106+
goto cleanup;
31213107
}
31223108

31233109
if (FFI_G(symbols)) {
@@ -3130,7 +3116,7 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
31303116
addr = DL_FETCH_SYMBOL(handle, ZSTR_VAL(name));
31313117
if (!addr) {
31323118
zend_throw_error(zend_ffi_exception_ce, "Failed resolving C variable '%s'", ZSTR_VAL(name));
3133-
RETURN_THROWS();
3119+
goto cleanup;
31343120
}
31353121
sym->addr = addr;
31363122
} else if (sym->kind == ZEND_FFI_SYM_FUNC) {
@@ -3140,7 +3126,7 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
31403126
zend_string_release(mangled_name);
31413127
if (!addr) {
31423128
zend_throw_error(zend_ffi_exception_ce, "Failed resolving C function '%s'", ZSTR_VAL(name));
3143-
RETURN_THROWS();
3129+
goto cleanup;
31443130
}
31453131
sym->addr = addr;
31463132
}
@@ -3157,6 +3143,22 @@ ZEND_METHOD(FFI, cdef) /* {{{ */
31573143
FFI_G(tags) = NULL;
31583144

31593145
RETURN_OBJ(&ffi->std);
3146+
3147+
cleanup:
3148+
if (lib && handle) {
3149+
DL_UNLOAD(handle);
3150+
}
3151+
if (FFI_G(symbols)) {
3152+
zend_hash_destroy(FFI_G(symbols));
3153+
efree(FFI_G(symbols));
3154+
FFI_G(symbols) = NULL;
3155+
}
3156+
if (FFI_G(tags)) {
3157+
zend_hash_destroy(FFI_G(tags));
3158+
efree(FFI_G(tags));
3159+
FFI_G(tags) = NULL;
3160+
}
3161+
RETURN_THROWS();
31603162
}
31613163
/* }}} */
31623164

ext/ffi/tests/gh14286_2.phpt

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,10 +2,6 @@
22
GH-14286 (ffi enum type (when enum has no name) make memory leak)
33
--EXTENSIONS--
44
ffi
5-
--SKIPIF--
6-
<?php
7-
if (PHP_DEBUG || getenv('SKIP_ASAN')) die("xfail: FFI cleanup after parser error is not implemented");
8-
?>
95
--INI--
106
ffi.enable=1
117
--FILE--
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
--TEST--
2+
GH-18629 (FFI::cdef() leaks on function resolution failure)
3+
--EXTENSIONS--
4+
ffi
5+
--INI--
6+
ffi.enable=1
7+
--FILE--
8+
<?php
9+
try {
10+
$ffi = FFI::cdef("void nonexistent_ffi_test_func(void);");
11+
} catch (\FFI\Exception $e) {
12+
echo $e->getMessage() . "\n";
13+
}
14+
?>
15+
--EXPECT--
16+
Failed resolving C function 'nonexistent_ffi_test_func'
Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
--TEST--
2+
GH-18629 (Read field of non C struct/union)
3+
--EXTENSIONS--
4+
ffi
5+
--INI--
6+
ffi.enable=1
7+
--FILE--
8+
<?php
9+
$x = FFI::cdef()->new("int*");
10+
try {
11+
$y = $x->foo;
12+
} catch (\FFI\Exception $e) {
13+
echo $e->getMessage() . "\n";
14+
}
15+
?>
16+
--EXPECT--
17+
Attempt to read field 'foo' of non C struct/union

0 commit comments

Comments
 (0)