From 6a8d74d6e14290b3c901fb821bd645a15c8f1ecd Mon Sep 17 00:00:00 2001 From: reeze Date: Mon, 30 Apr 2012 11:37:22 +0800 Subject: [PATCH 1/2] Fixed bug #61828 Memleak when calling Directory(Recursive)Iterator/Spl(Temp)FileObject ctor twice --- ext/spl/spl_directory.c | 22 ++++++++++++++++++++++ ext/spl/tests/bug61828.phpt | 26 ++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) create mode 100644 ext/spl/tests/bug61828.phpt diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index aaa256de7b27c..520d9d2940587 100755 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -708,6 +708,12 @@ 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; + + if (intern->_path) { + efree(intern->_path); + php_stream_close(intern->u.dir.dirp); + } + #ifdef HAVE_GLOB if (SPL_HAS_FLAG(ctor_flags, DIT_CTOR_GLOB) && strstr(path, "glob://") != path) { spprintf(&path, 0, "glob://%s", path); @@ -2285,6 +2291,14 @@ SPL_METHOD(SplFileObject, __construct) zend_replace_error_handling(EH_THROW, spl_ce_RuntimeException, &error_handling TSRMLS_CC); + if (intern->file_name) { + php_stream_close(intern->u.dir.dirp); + efree(intern->_path); + efree(intern->file_name); + efree(intern->orig_path); + efree(intern->u.file.open_mode); + } + intern->u.file.open_mode = NULL; intern->u.file.open_mode_len = 0; @@ -2349,6 +2363,14 @@ SPL_METHOD(SplTempFileObject, __construct) return; } + if (intern->file_name) { + 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; diff --git a/ext/spl/tests/bug61828.phpt b/ext/spl/tests/bug61828.phpt new file mode 100644 index 0000000000000..40789245a5830 --- /dev/null +++ b/ext/spl/tests/bug61828.phpt @@ -0,0 +1,26 @@ +--TEST-- +Testing Directory(Recursive)Iterator/Spl(Temp)FileObject ctor twice +--FILE-- +__construct(".."); + +$x = new RecursiveDirectoryIterator("."); +$x->__construct(".."); + +$x = new FilesystemIterator("."); +$x->__construct(".."); + +$x = new GlobIterator("."); +$x->__construct(".."); + +$x = new SplFileObject('.'); +$x->__construct('..'); + +$x = new SplTempFileObject(1); +$x->__construct(1); + +echo "done!\n"; +?> +--EXPECT-- +done! \ No newline at end of file From 3b3c4cfa7cc297c71ca25ea5d1148867337791d2 Mon Sep 17 00:00:00 2001 From: reeze Date: Tue, 1 May 2012 16:38:49 +0800 Subject: [PATCH 2/2] Fixed memleak/segfault when multiple construct Directory(Recursive)Iterator/Spl(Temp)FileObject --- ext/spl/spl_directory.c | 70 +++++++++++++++++++++++++++++-------- ext/spl/tests/bug61828.phpt | 53 +++++++++++++++++++++++++--- 2 files changed, 103 insertions(+), 20 deletions(-) diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 520d9d2940587..8dc64232af228 100755 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -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) */ @@ -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; @@ -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); @@ -709,10 +715,11 @@ 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; - if (intern->_path) { - efree(intern->_path); - php_stream_close(intern->u.dir.dirp); - } + /* 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) { @@ -723,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); } @@ -2285,19 +2306,19 @@ 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); - if (intern->file_name) { - php_stream_close(intern->u.dir.dirp); - efree(intern->_path); - efree(intern->file_name); - efree(intern->orig_path); - efree(intern->u.file.open_mode); - } + /* 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; @@ -2343,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); } /* }}} */ @@ -2363,7 +2403,7 @@ SPL_METHOD(SplTempFileObject, __construct) return; } - if (intern->file_name) { + if (intern->_path) { php_stream_close(intern->u.dir.dirp); efree(intern->_path); efree(intern->file_name); diff --git a/ext/spl/tests/bug61828.phpt b/ext/spl/tests/bug61828.phpt index 40789245a5830..231aefb80a4dc 100644 --- a/ext/spl/tests/bug61828.phpt +++ b/ext/spl/tests/bug61828.phpt @@ -1,7 +1,8 @@ --TEST-- -Testing Directory(Recursive)Iterator/Spl(Temp)FileObject ctor twice +Bug #61828 Directory(Recursive)Iterator/Spl(Temp)FileObject ctor twice cause memleak&segfault --FILE-- __construct(".."); @@ -14,13 +15,55 @@ $x->__construct(".."); $x = new GlobIterator("."); $x->__construct(".."); -$x = new SplFileObject('.'); -$x->__construct('..'); +$x = new SplFileObject(__FILE__); +$x->__construct(__FILE__); $x = new SplTempFileObject(1); $x->__construct(1); -echo "done!\n"; +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! \ No newline at end of file +DONE testing normal double construct +DONE testing double construct with later one thrown exception \ No newline at end of file