Skip to content

Fix GH-20286: use-after-destroy during userland stream_close(). #20348

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 4 commits into
base: PHP-8.3
Choose a base branch
from
Draft

Fix GH-20286: use-after-destroy during userland stream_close(). #20348

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
d354d83
Fix GH-20286: use-after-destroy during userland stream_close().
devnexen Oct 31, 2025
f4280b1
fix segfault on streams destruction
devnexen Oct 31, 2025
eb466e1
fix test
devnexen Oct 31, 2025
40424c5
test feedback
devnexen Nov 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions Zend/zend_execute_API.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1077,6 +1077,14 @@ ZEND_API zend_result zend_call_method_if_exists(
zend_object *object, zend_string *method_name, zval *retval,
uint32_t param_count, zval *params)
{
if (UNEXPECTED(!EG(active))) {
#if ZEND_DEBUG
ZEND_UNREACHABLE();
#endif
ZVAL_UNDEF(retval);
return FAILURE;
Copy link
Member

@iluuu1994 iluuu1994 Oct 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There should probably be a ZVAL_UNDEF(retval); as below.

}

zend_fcall_info fci;
fci.size = sizeof(zend_fcall_info);
fci.object = object;
Expand Down
58 changes: 58 additions & 0 deletions ext/standard/tests/streams/gh20286.phpt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
--TEST--
GH-20286 use after destroy on userland stream_close
--CREDITS--
vi3tL0u1s
--SKIPIF--
<?php
if (PHP_DEBUG) die('skip requires release build');
?>
--FILE--
<?php
class lib {
function stream_open() {
Copy link
Member

@iluuu1994 iluuu1994 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you indent by 4 spaces?

return true;
}

function stream_read($count) {
function a() {}
include('lib://');
}

function stream_close() {
include('lib://');
}
}
stream_wrapper_register('lib', lib::class);
include('lib://test.php');
?>
--EXPECTF--

Deprecated: Creation of dynamic property lib::$context is deprecated in %s on line %d
Copy link
Member

@iluuu1994 iluuu1994 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe create the $context prop and create empty stubs for these functions to make the warnings go away.


Warning: include(): lib::stream_set_option is not implemented! in %s on line %d

Warning: include(): lib::stream_stat is not implemented! in %s on line %d

Deprecated: Creation of dynamic property lib::$context is deprecated in %s on line %d

Warning: include(): lib::stream_set_option is not implemented! in %s on line %d

Warning: include(): lib::stream_stat is not implemented! in %s on line %d

Fatal error: Cannot redeclare a() (previously declared in %s:%d) %s on line %d

Deprecated: Creation of dynamic property lib::$context is deprecated in %s on line %d

Warning: include(): lib::stream_set_option is not implemented! in %s on line %d

Warning: include(): lib::stream_stat is not implemented! in %s on line %d

Fatal error: Cannot redeclare a() (previously declared in %s:%d) %s on line %d

Deprecated: Creation of dynamic property lib::$context is deprecated in %s on line %d

Warning: include(): lib::stream_set_option is not implemented! in %s on line %d

Warning: include(): lib::stream_stat is not implemented! in %s on line %d

Fatal error: Cannot redeclare a() (previously declared in %s:%d) %s on line %d
4 changes: 4 additions & 0 deletions main/streams/userspace.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -695,6 +695,10 @@ static int php_userstreamop_close(php_stream *stream, int close_handle)

assert(us != NULL);

if (UNEXPECTED(stream->wrapper->wops != &user_stream_wops)) {
stream->wrapper->wops = &user_stream_wops;
}
Copy link
Member

@iluuu1994 iluuu1994 Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks a bit odd, why is this necessary?

Copy link
Member Author

@devnexen devnexen Nov 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

Program received signal SIGSEGV, Segmentation fault.
0x0000555555aa0f35 in _php_stream_free (stream=<optimized out>, close_options=close_options@entry=11) at /home/dcarlier/Contribs/php-src/main/streams/streams.c:505
505				stream->wrapper->wops->stream_closer(stream->wrapper, stream);
(gdb) p *stream->wrapper->wops
$2 = {stream_opener = 0x7ffff767be60, stream_closer = 0xc0d29af9ee47b9cc, stream_stat = 0xb, url_stat = 0x735f6d6165727473, dir_opener = 0x6f69747000746174, 
  label = 0x6e <error: Cannot access memory at address 0x6e>, unlink = 0x0, rename = 0x5555568bc500, stream_mkdir = 0x0, stream_rmdir = 0x5a0, stream_metadata = 0x7ffff767beb0}
(gdb) p *stream->wrapper->wops->stream_closer
Cannot access memory at address 0xc0d29af9ee47b9cc
(gdb)

Copy link
Member

@ndossche ndossche Nov 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because first the new resource is created using the wrapper, and then in the same loop the wrapper is destroyed without destroying the resource first. Later on, we try to destroy the resource which will deref the wrapper.
This is wrong though, I put out an alternative PR as David requested help here.


ZVAL_STRINGL(&func_name, USERSTREAM_CLOSE, sizeof(USERSTREAM_CLOSE)-1);

call_method_if_exists(&us->object, &func_name, &retval, 0, NULL);
Expand Down
Loading