From 4d9a1883aa764e502990488d2e8b9c978be6fbd2 Mon Sep 17 00:00:00 2001 From: Dmitry Stogov Date: Wed, 8 Apr 2015 13:29:42 +0300 Subject: [PATCH] Fixed bug #68887 (resources are not freed correctly) --- Zend/tests/bug68887.phpt | 14 ++++++++++++++ ext/bz2/bz2.c | 3 +++ ext/mysqlnd/mysqlnd_auth.c | 2 +- ext/mysqlnd/mysqlnd_debug.c | 2 +- ext/sockets/sockets.c | 3 ++- ext/spl/spl_directory.c | 4 ++-- ext/standard/file.c | 8 +++----- ext/standard/filestat.c | 2 +- main/php_streams.h | 1 + main/streams/cast.c | 4 ++-- main/streams/streams.c | 11 +++-------- 11 files changed, 33 insertions(+), 21 deletions(-) create mode 100644 Zend/tests/bug68887.phpt diff --git a/Zend/tests/bug68887.phpt b/Zend/tests/bug68887.phpt new file mode 100644 index 00000000000..2c2a0211c02 --- /dev/null +++ b/Zend/tests/bug68887.phpt @@ -0,0 +1,14 @@ +--TEST-- +Bug #68887 (resources are not freed correctly) +--FILE-- + +--EXPECT-- +bool(true) +bool(true) diff --git a/ext/bz2/bz2.c b/ext/bz2/bz2.c index 921fc9954b1..d4175c724a5 100644 --- a/ext/bz2/bz2.c +++ b/ext/bz2/bz2.c @@ -226,6 +226,9 @@ PHP_BZ2_API php_stream *_php_stream_bz2open_from_BZFILE(BZFILE *bz, self = emalloc(sizeof(*self)); self->stream = innerstream; + if (innerstream) { + GC_REFCOUNT(innerstream->res)++; + } self->bz_file = bz; return php_stream_alloc_rel(&php_stream_bz2io_ops, self, 0, mode); diff --git a/ext/mysqlnd/mysqlnd_auth.c b/ext/mysqlnd/mysqlnd_auth.c index e9eb773f9ca..2ecdd95ede8 100644 --- a/ext/mysqlnd/mysqlnd_auth.c +++ b/ext/mysqlnd/mysqlnd_auth.c @@ -555,7 +555,7 @@ mysqlnd_sha256_get_rsa_key(MYSQLND_CONN_DATA * conn, DBG_INF_FMT("Public key:%*.s", key_str->len, key_str->val); zend_string_release(key_str); } - php_stream_free(stream, PHP_STREAM_FREE_CLOSE); + php_stream_close(stream); } } DBG_RETURN(ret); diff --git a/ext/mysqlnd/mysqlnd_debug.c b/ext/mysqlnd/mysqlnd_debug.c index 812963ae1ae..790ac00cec1 100644 --- a/ext/mysqlnd/mysqlnd_debug.c +++ b/ext/mysqlnd/mysqlnd_debug.c @@ -460,7 +460,7 @@ MYSQLND_METHOD(mysqlnd_debug, close)(MYSQLND_DEBUG * self) } #endif - php_stream_free(self->stream, PHP_STREAM_FREE_CLOSE); + php_stream_close(self->stream); self->stream = NULL; } /* no DBG_RETURN please */ diff --git a/ext/sockets/sockets.c b/ext/sockets/sockets.c index 45f7c250962..19e1c76d388 100644 --- a/ext/sockets/sockets.c +++ b/ext/sockets/sockets.c @@ -1079,7 +1079,8 @@ PHP_FUNCTION(socket_close) if (stream != NULL) { /* close & destroy stream, incl. removing it from the rsrc list; * resource stored in php_sock->zstream will become invalid */ - php_stream_free(stream, PHP_STREAM_FREE_CLOSE | + php_stream_free(stream, + PHP_STREAM_FREE_KEEP_RSRC | PHP_STREAM_FREE_CLOSE | (stream->is_persistent?PHP_STREAM_FREE_CLOSE_PERSISTENT:0)); } } diff --git a/ext/spl/spl_directory.c b/ext/spl/spl_directory.c index 3e250b1accf..12c50c0b463 100644 --- a/ext/spl/spl_directory.c +++ b/ext/spl/spl_directory.c @@ -108,9 +108,9 @@ static void spl_filesystem_object_free_storage(zend_object *object) /* {{{ */ } */ if (!intern->u.file.stream->is_persistent) { - php_stream_free(intern->u.file.stream, PHP_STREAM_FREE_CLOSE); + php_stream_close(intern->u.file.stream); } else { - php_stream_free(intern->u.file.stream, PHP_STREAM_FREE_CLOSE_PERSISTENT); + php_stream_pclose(intern->u.file.stream); } if (intern->u.file.open_mode) { efree(intern->u.file.open_mode); diff --git a/ext/standard/file.c b/ext/standard/file.c index f31407b7575..fa9e752dc62 100644 --- a/ext/standard/file.c +++ b/ext/standard/file.c @@ -900,11 +900,9 @@ PHPAPI PHP_FUNCTION(fclose) RETURN_FALSE; } - if (!stream->is_persistent) { - php_stream_close(stream); - } else { - php_stream_pclose(stream); - } + php_stream_free(stream, + PHP_STREAM_FREE_KEEP_RSRC | + (stream->is_persistent ? PHP_STREAM_FREE_CLOSE_PERSISTENT : PHP_STREAM_FREE_CLOSE)); RETURN_TRUE; } diff --git a/ext/standard/filestat.c b/ext/standard/filestat.c index 54bc5813e71..ffcba3598d1 100644 --- a/ext/standard/filestat.c +++ b/ext/standard/filestat.c @@ -765,7 +765,7 @@ PHP_FUNCTION(touch) } stream = php_stream_open_wrapper_ex(filename, "c", REPORT_ERRORS, NULL, NULL); if(stream != NULL) { - php_stream_pclose(stream); + php_stream_close(stream); RETURN_TRUE; } else { RETURN_FALSE; diff --git a/main/php_streams.h b/main/php_streams.h index 4f3d860a906..0ee3ff54588 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -284,6 +284,7 @@ PHPAPI int php_stream_from_persistent_id(const char *persistent_id, php_stream * #define PHP_STREAM_FREE_RSRC_DTOR 8 /* called from the resource list dtor */ #define PHP_STREAM_FREE_PERSISTENT 16 /* manually freeing a persistent connection */ #define PHP_STREAM_FREE_IGNORE_ENCLOSING 32 /* don't close the enclosing stream instead */ +#define PHP_STREAM_FREE_KEEP_RSRC 64 /* keep associated zend_resource */ #define PHP_STREAM_FREE_CLOSE (PHP_STREAM_FREE_CALL_DTOR | PHP_STREAM_FREE_RELEASE_STREAM) #define PHP_STREAM_FREE_CLOSE_CASTED (PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_PRESERVE_HANDLE) #define PHP_STREAM_FREE_CLOSE_PERSISTENT (PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_PERSISTENT) diff --git a/main/streams/cast.c b/main/streams/cast.c index bc9cc805e8f..23c59c71c09 100644 --- a/main/streams/cast.c +++ b/main/streams/cast.c @@ -86,7 +86,7 @@ static int stream_cookie_closer(void *cookie) /* prevent recursion */ stream->fclose_stdiocast = PHP_STREAM_FCLOSE_NONE; - return php_stream_close(stream); + return php_stream_free(stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_KEEP_RSRC); } #elif defined(HAVE_FOPENCOOKIE) static ssize_t stream_cookie_reader(void *cookie, char *buffer, size_t size) @@ -128,7 +128,7 @@ static int stream_cookie_closer(void *cookie) /* prevent recursion */ stream->fclose_stdiocast = PHP_STREAM_FCLOSE_NONE; - return php_stream_close(stream); + return php_stream_free(stream, PHP_STREAM_FREE_CLOSE | PHP_STREAM_FREE_KEEP_RSRC); } #endif /* elif defined(HAVE_FOPENCOOKIE) */ diff --git a/main/streams/streams.c b/main/streams/streams.c index 886fc1da289..2358cc9df3e 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -439,15 +439,10 @@ fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remov /* If not called from the resource dtor, remove the stream from the resource list. */ if ((close_options & PHP_STREAM_FREE_RSRC_DTOR) == 0 && stream->res) { - /* zend_list_delete actually only decreases the refcount; if we're - * releasing the stream, we want to actually delete the resource from - * the resource list, otherwise the resource will point to invalid memory. - * In any case, let's always completely delete it from the resource list, - * not only when PHP_STREAM_FREE_RELEASE_STREAM is set */ -//??? while (zend_list_delete(stream->res) == SUCCESS) {} -//??? stream->res->gc.refcount = 0; + /* Close resource, but keep it in resource list */ zend_list_close(stream->res); - if (!stream->__exposed) { + if ((close_options & PHP_STREAM_FREE_KEEP_RSRC) == 0) { + /* Completely delete zend_resource, if not referenced */ zend_list_delete(stream->res); stream->res = NULL; }