Skip to content

Fixed bug #61828 Memleak when calling Directory(Recursive)Iterator/Spl(T... #71

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.

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

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
68 changes: 65 additions & 3 deletions ext/spl/spl_directory.c
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,9 @@ static void spl_filesystem_dir_open(spl_filesystem_object* intern, char *path TS
intern->u.dir.index = 0;

if (EG(exception) || intern->u.dir.dirp == NULL) {
if (intern->_path_len > 0) {
efree(intern->_path);
}
intern->u.dir.entry.d_name[0] = '\0';
if (!EG(exception)) {
/* open failed w/out notice (turned to exception due to EH_THROW) */
Expand Down Expand Up @@ -290,6 +293,7 @@ static int spl_filesystem_file_open(spl_filesystem_object *intern, int use_inclu
if (!EG(exception)) {
zend_throw_exception_ex(spl_ce_RuntimeException, 0 TSRMLS_CC, "Cannot open file '%s'", intern->file_name_len ? intern->file_name : "");
}
intern->_path = NULL;
intern->file_name = NULL; /* until here it is not a copy */
intern->u.file.open_mode = NULL;
return FAILURE;
Expand Down Expand Up @@ -676,9 +680,11 @@ zend_function *spl_filesystem_object_get_method_check(zval **object_ptr, char *m
void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, long ctor_flags) /* {{{ */
{
spl_filesystem_object *intern;
char *path;
char *path, *orig_path, *orig_intern_path;
int parsed, len;
long flags;
php_stream *orig_stream;
php_stream_dirent orig_dirent;
zend_error_handling error_handling;

zend_replace_error_handling(EH_THROW, spl_ce_UnexpectedValueException, &error_handling TSRMLS_CC);
Expand Down Expand Up @@ -708,6 +714,13 @@ void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, long ctor_fla

intern = (spl_filesystem_object*)zend_object_store_get_object(getThis() TSRMLS_CC);
intern->flags = flags;

/* backup state for later recovery in case of exception thrown when double construct */
orig_path = intern->_path;
orig_intern_path = intern->orig_path;
orig_stream = intern->u.dir.dirp;
orig_dirent = intern->u.dir.entry;

#ifdef HAVE_GLOB
if (SPL_HAS_FLAG(ctor_flags, DIT_CTOR_GLOB) && strstr(path, "glob://") != path) {
spprintf(&path, 0, "glob://%s", path);
Expand All @@ -717,10 +730,24 @@ void spl_filesystem_object_construct(INTERNAL_FUNCTION_PARAMETERS, long ctor_fla
#endif
{
spl_filesystem_dir_open(intern, path TSRMLS_CC);
}

if (!EG(exception)) {
intern->u.dir.is_recursive = instanceof_function(intern->std.ce, spl_ce_RecursiveDirectoryIterator TSRMLS_CC) ? 1 : 0;
}

intern->u.dir.is_recursive = instanceof_function(intern->std.ce, spl_ce_RecursiveDirectoryIterator TSRMLS_CC) ? 1 : 0;
if (orig_path) {
if (EG(exception)) {
intern->_path = orig_path;
intern->_path_len = strlen(orig_path);
intern->u.dir.dirp = orig_stream;
intern->u.dir.entry = orig_dirent;
intern->orig_path = orig_intern_path;
} else {
php_stream_close(orig_stream);
efree(orig_path);
}
}

zend_restore_error_handling(&error_handling TSRMLS_CC);
}
Expand Down Expand Up @@ -2279,12 +2306,20 @@ SPL_METHOD(SplFileObject, __construct)
spl_filesystem_object *intern = (spl_filesystem_object*)zend_object_store_get_object(getThis() TSRMLS_CC);
zend_bool use_include_path = 0;
char *p1, *p2;
char *tmp_path;
char *tmp_path, *orig_path, *orig_file_name, *orig_intern_path, *orig_open_mode;
int tmp_path_len;
php_stream *orig_stream;
zend_error_handling error_handling;

zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling TSRMLS_CC);

/* backup state for later recovery in case of exception thrown when double construct */
orig_stream = intern->u.file.stream;
orig_path = intern->_path;
orig_intern_path = intern->orig_path;
orig_file_name = intern->file_name;
orig_open_mode = intern->u.file.open_mode;

intern->u.file.open_mode = NULL;
intern->u.file.open_mode_len = 0;

Expand Down Expand Up @@ -2329,6 +2364,25 @@ SPL_METHOD(SplFileObject, __construct)
intern->_path = estrndup(intern->u.file.stream->orig_path, intern->_path_len);
}

if (orig_path) {
if (EG(exception)) {
intern->u.file.stream = orig_stream;
intern->_path = orig_path;
intern->_path_len = strlen(orig_path);
intern->file_name = orig_file_name;
intern->file_name_len = strlen(orig_file_name);
intern->orig_path = orig_intern_path;
intern->u.file.open_mode = orig_open_mode;
intern->u.file.open_mode_len = strlen(orig_open_mode);
} else {
php_stream_close(orig_stream);
efree(orig_path);
efree(orig_file_name);
efree(orig_intern_path);
efree(orig_open_mode);
}
}

zend_restore_error_handling(&error_handling TSRMLS_CC);

} /* }}} */
Expand All @@ -2349,6 +2403,14 @@ SPL_METHOD(SplTempFileObject, __construct)
return;
}

if (intern->_path) {
php_stream_close(intern->u.dir.dirp);
efree(intern->_path);
efree(intern->file_name);
efree(intern->orig_path);
efree(intern->u.file.open_mode);
}

if (max_memory < 0) {
intern->file_name = "php://memory";
intern->file_name_len = 12;
Expand Down
69 changes: 69 additions & 0 deletions ext/spl/tests/bug61828.phpt
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
--TEST--
Bug #61828 Directory(Recursive)Iterator/Spl(Temp)FileObject ctor twice cause memleak&segfault
--FILE--
<?php
/* Leak testing */
$x = new DirectoryIterator(".");
$x->__construct("..");

$x = new RecursiveDirectoryIterator(".");
$x->__construct("..");

$x = new FilesystemIterator(".");
$x->__construct("..");

$x = new GlobIterator(".");
$x->__construct("..");

$x = new SplFileObject(__FILE__);
$x->__construct(__FILE__);

$x = new SplTempFileObject(1);
$x->__construct(1);

echo "DONE testing normal double construct\n";

/*
* When trying to get debug info by convert to string it will segfault.
* So here we simply trigger it by type convertion
*/
$y = new DirectoryIterator(".");
try {
$y->__construct("bug-path");
} catch (Exception $e)
{}
(string)$y;

$y = new RecursiveDirectoryIterator(".");
try {
$y->__construct("bug-path");
} catch (Exception $e)
{}
(string)$y;

$y = new FilesystemIterator(".");
try {
$y->__construct("bug-path");
} catch (Exception $e)
{}
(string)$y;

$y = new SplFileObject(__FILE__);
try {
$y->__construct('bug-path');
} catch (Exception $e)
{}
(string)$y;

$y = new SplTempFileObject(1);
try {
$y->__construct('bug-param');
} catch (Exception $e)
{}
(string)$y;

echo "DONE testing double construct with later one thrown exception\n";
?>
--EXPECT--
DONE testing normal double construct
DONE testing double construct with later one thrown exception