diff --git a/NEWS b/NEWS index 217d6b959c81..6df11a156e18 100644 --- a/NEWS +++ b/NEWS @@ -51,6 +51,10 @@ PHP NEWS TCP_USER_TIMEOUT, and SO_LINGER options. (Weilin Du) . Fixed various memory related issues in ext/sockets. (David Carlier) +- Standard: + . Fixed bug GH-22844 (Use-after-free in StreamPollHandle after the + underlying stream is closed). (iliaal, arnaud-lb) + - Streams: . Added a new IO copy API used by php_stream_copy_to_stream_ex() that leverages platform primitives (sendfile, splice, copy_file_range, diff --git a/ext/standard/io_poll.c b/ext/standard/io_poll.c index ca363c7b37d9..80ee8ce4f4e6 100644 --- a/ext/standard/io_poll.c +++ b/ext/standard/io_poll.c @@ -17,6 +17,7 @@ #include "zend_exceptions.h" #include "php_network.h" #include "php_poll.h" +#include "io_poll.h" #include "io_poll_arginfo.h" #include "io_poll_decl.h" @@ -54,6 +55,8 @@ typedef struct php_io_poll_watcher_object { zval data; bool active; php_io_poll_context_object *context; /* Back reference to Context object */ + php_socket_t fd; + php_stream *stream; zend_object std; } php_io_poll_watcher_object; @@ -66,7 +69,6 @@ struct php_io_poll_context_object { /* Stream poll handle specific data */ typedef struct php_stream_poll_handle_data { - php_stream *stream; zend_resource *res; } php_stream_poll_handle_data; @@ -179,16 +181,24 @@ static const char *php_io_poll_backend_type_to_name(php_poll_backend_type type) /* Stream Poll Handle Implementation */ +static zend_always_inline php_stream *php_stream_poll_handle_get_stream(php_stream_poll_handle_data *data) +{ + if (!data || !data->res) { + return NULL; + } + return (php_stream *) zend_fetch_resource2(data->res, NULL, php_file_le_stream(), php_file_le_pstream()); +} + static php_socket_t php_stream_poll_handle_get_fd(php_poll_handle_object *handle) { - php_stream_poll_handle_data *data = handle->handle_data; + php_stream *stream = php_stream_poll_handle_get_stream(handle->handle_data); php_socket_t fd; - if (!data || !data->stream) { + if (!stream) { return SOCK_ERR; } - if (php_stream_cast(data->stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, + if (php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL, (void *) &fd, 1) != SUCCESS || fd == -1) { @@ -200,8 +210,8 @@ static php_socket_t php_stream_poll_handle_get_fd(php_poll_handle_object *handle static int php_stream_poll_handle_is_valid(php_poll_handle_object *handle) { - php_stream_poll_handle_data *data = handle->handle_data; - return data && data->stream && !php_stream_eof(data->stream); + php_stream *stream = php_stream_poll_handle_get_stream(handle->handle_data); + return stream && !php_stream_eof(stream); } static void php_stream_poll_handle_cleanup(php_poll_handle_object *handle) @@ -254,6 +264,8 @@ static zend_object *php_io_poll_watcher_create_object(zend_class_entry *ce) intern->triggered_events = 0; intern->active = false; intern->context = NULL; + intern->fd = SOCK_ERR; + intern->stream = NULL; ZVAL_NULL(&intern->data); return &intern->std; @@ -272,12 +284,94 @@ static zend_object *php_io_poll_context_create_object(zend_class_entry *ce) return &intern->std; } +static zend_always_inline zend_ulong php_io_poll_compute_ptr_key(void *ptr) +{ + zend_ulong key = (zend_ulong) (uintptr_t) ptr; + return (key >> 3) | (key << ((sizeof(key) * 8) - 3)); +} + +static zend_always_inline php_stream *php_io_poll_watcher_get_stream(php_io_poll_watcher_object *watcher) +{ + if (!watcher->handle || watcher->handle->ops != &php_stream_poll_handle_ops) { + return NULL; + } + return php_stream_poll_handle_get_stream(watcher->handle->handle_data); +} + +static void php_io_poll_stream_watch(php_stream *stream, php_io_poll_watcher_object *watcher) +{ + if (!stream->poll_watchers) { + stream->poll_watchers = pemalloc(sizeof(HashTable), stream->is_persistent); + zend_hash_init(stream->poll_watchers, 4, NULL, NULL, stream->is_persistent); + } + + zval zv; + ZVAL_PTR(&zv, watcher); + zend_hash_index_update(stream->poll_watchers, php_io_poll_compute_ptr_key(watcher), &zv); +} + +static void php_io_poll_stream_unwatch(php_io_poll_watcher_object *watcher) +{ + php_stream *stream = watcher->stream; + watcher->stream = NULL; + + if (!stream || !stream->poll_watchers) { + return; + } + + zend_hash_index_del(stream->poll_watchers, php_io_poll_compute_ptr_key(watcher)); + + if (zend_hash_num_elements(stream->poll_watchers) == 0) { + zend_hash_destroy(stream->poll_watchers); + pefree(stream->poll_watchers, stream->is_persistent); + stream->poll_watchers = NULL; + } +} + +PHPAPI void php_io_poll_stream_notify_close(php_stream *stream) +{ + HashTable *watchers = stream->poll_watchers; + if (!watchers) { + return; + } + stream->poll_watchers = NULL; + + ZEND_HASH_FOREACH_VAL(watchers, zval *zv) { + GC_ADDREF(&((php_io_poll_watcher_object *) Z_PTR_P(zv))->std); + } ZEND_HASH_FOREACH_END(); + + ZEND_HASH_FOREACH_VAL(watchers, zval *zv) { + php_io_poll_watcher_object *watcher = Z_PTR_P(zv); + php_io_poll_context_object *context = watcher->context; + watcher->stream = NULL; + if (context) { + if (watcher->fd != SOCK_ERR && context->ctx) { + php_poll_remove(context->ctx, (int) watcher->fd); + } + watcher->active = false; + watcher->context = NULL; + if (context->watchers) { + zend_hash_index_del(context->watchers, php_io_poll_compute_ptr_key(watcher->handle)); + } + } + } ZEND_HASH_FOREACH_END(); + + ZEND_HASH_FOREACH_VAL(watchers, zval *zv) { + OBJ_RELEASE(&((php_io_poll_watcher_object *) Z_PTR_P(zv))->std); + } ZEND_HASH_FOREACH_END(); + + zend_hash_destroy(watchers); + pefree(watchers, stream->is_persistent); +} + /* Object Destruction Functions */ static void php_io_poll_watcher_free_object(zend_object *obj) { php_io_poll_watcher_object *intern = PHP_POLL_WATCHER_OBJ_FROM_ZOBJ(obj); + php_io_poll_stream_unwatch(intern); + zval_ptr_dtor(&intern->data); if (intern->handle) { @@ -294,6 +388,7 @@ static void php_io_poll_context_free_object(zend_object *obj) if (intern->watchers) { ZEND_HASH_FOREACH_VAL(intern->watchers, zval *zv) { php_io_poll_watcher_object *watcher = PHP_POLL_WATCHER_OBJ_FROM_ZOBJ(Z_OBJ_P(zv)); + php_io_poll_stream_unwatch(watcher); watcher->active = false; watcher->context = NULL; } ZEND_HASH_FOREACH_END(); @@ -340,14 +435,6 @@ static HashTable *php_io_poll_context_get_gc(zend_object *obj, zval **table, int return NULL; } -/* Utility functions */ - -static zend_always_inline zend_ulong php_io_poll_compute_ptr_key(void *ptr) -{ - zend_ulong key = (zend_ulong) (uintptr_t) ptr; - return (key >> 3) | (key << ((sizeof(key) * 8) - 3)); -} - static zend_result php_io_poll_watcher_modify_events( php_io_poll_watcher_object *watcher, uint32_t events) { @@ -357,7 +444,7 @@ static zend_result php_io_poll_watcher_modify_events( return FAILURE; } - php_socket_t fd = php_poll_handle_get_fd(watcher->handle); + php_socket_t fd = watcher->fd; if (fd == SOCK_ERR) { zend_throw_exception( php_io_poll_invalid_handle_class_entry, "Invalid handle for polling", 0); @@ -453,7 +540,6 @@ PHP_METHOD(StreamPollHandle, __construct) /* Set up stream-specific data */ php_stream_poll_handle_data *data = emalloc(sizeof(php_stream_poll_handle_data)); - data->stream = stream; data->res = stream->res; intern->handle_data = data; @@ -466,14 +552,14 @@ PHP_METHOD(StreamPollHandle, getStream) ZEND_PARSE_PARAMETERS_NONE(); php_poll_handle_object *intern = PHP_POLL_HANDLE_OBJ_FROM_ZV(getThis()); - php_stream_poll_handle_data *data = intern->handle_data; + php_stream *stream = php_stream_poll_handle_get_stream(intern->handle_data); - if (!data || !data->stream) { + if (!stream) { RETURN_NULL(); } - GC_ADDREF(data->stream->res); - php_stream_to_zval(data->stream, return_value); + GC_ADDREF(stream->res); + php_stream_to_zval(stream, return_value); } PHP_METHOD(StreamPollHandle, isValid) @@ -645,13 +731,15 @@ PHP_METHOD(Io_Poll_Watcher, remove) php_poll_ctx *poll_ctx = context->ctx; HashTable *watchers = context->watchers; zend_ulong hash_key = php_io_poll_compute_ptr_key(intern->handle); - php_socket_t fd = php_poll_handle_get_fd(intern->handle); - if (fd != SOCK_ERR) { - php_poll_remove(poll_ctx, (int) fd); + if (intern->fd != SOCK_ERR) { + php_poll_remove(poll_ctx, (int) intern->fd); } + php_io_poll_stream_unwatch(intern); + intern->active = false; intern->context = NULL; + intern->fd = SOCK_ERR; if (watchers) { zend_hash_index_del(watchers, hash_key); @@ -770,6 +858,13 @@ PHP_METHOD(Io_Poll_Context, add) watcher->active = true; watcher->context = intern; + watcher->fd = fd; + + php_stream *watched_stream = php_io_poll_watcher_get_stream(watcher); + if (watched_stream) { + watcher->stream = watched_stream; + php_io_poll_stream_watch(watched_stream, watcher); + } } PHP_METHOD(Io_Poll_Context, wait) diff --git a/ext/standard/io_poll.h b/ext/standard/io_poll.h new file mode 100644 index 000000000000..ae2e35b1cfc2 --- /dev/null +++ b/ext/standard/io_poll.h @@ -0,0 +1,24 @@ +/* + +----------------------------------------------------------------------+ + | Copyright © The PHP Group and Contributors. | + +----------------------------------------------------------------------+ + | This source file is subject to the Modified BSD License that is | + | bundled with this package in the file LICENSE, and is available | + | through the World Wide Web at . | + | | + | SPDX-License-Identifier: BSD-3-Clause | + +----------------------------------------------------------------------+ +*/ + +#ifndef PHP_IO_POLL_H +#define PHP_IO_POLL_H + +#include "php.h" + +BEGIN_EXTERN_C() + +PHPAPI void php_io_poll_stream_notify_close(struct _php_stream *stream); + +END_EXTERN_C() + +#endif /* PHP_IO_POLL_H */ diff --git a/ext/standard/io_poll.stub.php b/ext/standard/io_poll.stub.php index 83c1ba5cbe2e..cd7f5c699c08 100644 --- a/ext/standard/io_poll.stub.php +++ b/ext/standard/io_poll.stub.php @@ -159,7 +159,7 @@ final class StreamPollHandle implements Io\Poll\Handle /** @param resource $stream */ public function __construct($stream) {} - /** @return resource */ + /** @return resource|null */ public function getStream() {} public function isValid(): bool {} diff --git a/ext/standard/io_poll_arginfo.h b/ext/standard/io_poll_arginfo.h index 5fc62f629564..db23884778e0 100644 --- a/ext/standard/io_poll_arginfo.h +++ b/ext/standard/io_poll_arginfo.h @@ -1,5 +1,5 @@ /* This is a generated file, edit io_poll.stub.php instead. - * Stub hash: a7450146c5b3b3f3486611c83a55cf0cc932b27a + * Stub hash: 92428b6d5549f865148cf2b7c2b0f3f1e47a7726 * Has decl header: yes */ ZEND_BEGIN_ARG_WITH_RETURN_TYPE_INFO_EX(arginfo_class_Io_Poll_Backend_getAvailableBackends, 0, 0, IS_ARRAY, 0) diff --git a/ext/standard/io_poll_decl.h b/ext/standard/io_poll_decl.h index 2b09e3665f58..0a9394a745c2 100644 --- a/ext/standard/io_poll_decl.h +++ b/ext/standard/io_poll_decl.h @@ -1,8 +1,8 @@ /* This is a generated file, edit io_poll.stub.php instead. - * Stub hash: a7450146c5b3b3f3486611c83a55cf0cc932b27a */ + * Stub hash: 92428b6d5549f865148cf2b7c2b0f3f1e47a7726 */ -#ifndef ZEND_IO_POLL_DECL_a7450146c5b3b3f3486611c83a55cf0cc932b27a_H -#define ZEND_IO_POLL_DECL_a7450146c5b3b3f3486611c83a55cf0cc932b27a_H +#ifndef ZEND_IO_POLL_DECL_92428b6d5549f865148cf2b7c2b0f3f1e47a7726_H +#define ZEND_IO_POLL_DECL_92428b6d5549f865148cf2b7c2b0f3f1e47a7726_H typedef enum zend_enum_Io_Poll_Backend { ZEND_ENUM_Io_Poll_Backend_Auto = 1, @@ -23,4 +23,4 @@ typedef enum zend_enum_Io_Poll_Event { ZEND_ENUM_Io_Poll_Event_EdgeTriggered = 7, } zend_enum_Io_Poll_Event; -#endif /* ZEND_IO_POLL_DECL_a7450146c5b3b3f3486611c83a55cf0cc932b27a_H */ +#endif /* ZEND_IO_POLL_DECL_92428b6d5549f865148cf2b7c2b0f3f1e47a7726_H */ diff --git a/ext/standard/tests/poll/gh22844.phpt b/ext/standard/tests/poll/gh22844.phpt new file mode 100644 index 000000000000..d6592df7c5db --- /dev/null +++ b/ext/standard/tests/poll/gh22844.phpt @@ -0,0 +1,39 @@ +--TEST-- +GH-22844 (StreamPollHandle accessors are safe after the stream is closed) +--FILE-- +isValid()); +echo "getStream is resource: "; var_dump(is_resource($handle->getStream())); + +fclose($r); +fclose($w); + +echo "after close:\n"; +echo "isValid: "; var_dump($handle->isValid()); +echo "getStream: "; var_dump($handle->getStream()); + +try { + $context->add($handle, [Io\Poll\Event::Read]); + echo "add: no exception\n"; +} catch (\Throwable $e) { + echo "add: ", $e::class, ": ", $e->getMessage(), "\n"; +} + +echo "done\n"; +?> +--EXPECT-- +before close: +isValid: bool(true) +getStream is resource: bool(true) +after close: +isValid: bool(false) +getStream: NULL +add: Io\Poll\InvalidHandleException: Invalid handle for polling +done diff --git a/ext/standard/tests/poll/poll_stream_close_dup_fd.phpt b/ext/standard/tests/poll/poll_stream_close_dup_fd.phpt new file mode 100644 index 000000000000..07ec3131c90a --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_close_dup_fd.phpt @@ -0,0 +1,38 @@ +--TEST-- +Io\Poll: closing a watched stream whose fd is duplicated removes the registration (GH-22844) +--SKIPIF-- +isAvailable()) { + die("skip Epoll backend required\n"); +} +?> +--FILE-- +add(new StreamPollHandle($r), [Io\Poll\Event::Read]); + +$proc = proc_open('sleep 1', [0 => $r], $pipes); +fclose($r); +unset($watcher); +gc_collect_cycles(); + +fwrite($w, "ping"); +echo "events: "; var_dump(count($ctx->wait(0, 0))); + +proc_terminate($proc); +proc_close($proc); +echo "done\n"; +?> +--EXPECT-- +events: int(0) +done diff --git a/ext/standard/tests/poll/poll_stream_close_multi_context.phpt b/ext/standard/tests/poll/poll_stream_close_multi_context.phpt new file mode 100644 index 000000000000..6c57c898a858 --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_close_multi_context.phpt @@ -0,0 +1,28 @@ +--TEST-- +Io\Poll: closing a stream watched in multiple contexts retires every watcher (GH-22844) +--FILE-- +add($handle, [Io\Poll\Event::Read]); +$w2 = $ctx2->add($handle, [Io\Poll\Event::Read]); + +fwrite($w, "ping"); +fclose($r); +fclose($w); + +echo "ctx1: "; var_dump(count($ctx1->wait(0, 0))); +echo "ctx2: "; var_dump(count($ctx2->wait(0, 0))); +echo "done\n"; +?> +--EXPECT-- +ctx1: int(0) +ctx2: int(0) +done diff --git a/ext/standard/tests/poll/poll_stream_close_persistent.phpt b/ext/standard/tests/poll/poll_stream_close_persistent.phpt new file mode 100644 index 000000000000..15401ea660ec --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_close_persistent.phpt @@ -0,0 +1,37 @@ +--TEST-- +Io\Poll: a watcher on a persistent stream is unregistered at shutdown (GH-22844) +--SKIPIF-- +isAvailable()) { + die("skip Poll backend not available\n"); +} +$srv = @stream_socket_server("tcp://127.0.0.1:0", $e1, $e2); +if (!$srv) { + die("skip cannot bind loopback listener\n"); +} +$addr = stream_socket_get_name($srv, false); +$p = @pfsockopen("tcp://" . $addr, -1, $en, $es, 1); +if (!$p) { + die("skip pfsockopen unavailable\n"); +} +?> +--FILE-- +add(new StreamPollHandle($p), [Io\Poll\Event::Write]); + +echo "watched: "; var_dump(count($ctx->wait(0, 0)) >= 0); +echo "done\n"; +// $watcher / $ctx / persistent $p are intentionally left for shutdown teardown. +?> +--EXPECT-- +watched: bool(true) +done diff --git a/ext/standard/tests/poll/poll_stream_close_retires_watcher.phpt b/ext/standard/tests/poll/poll_stream_close_retires_watcher.phpt new file mode 100644 index 000000000000..ba6d089ac8ae --- /dev/null +++ b/ext/standard/tests/poll/poll_stream_close_retires_watcher.phpt @@ -0,0 +1,32 @@ +--TEST-- +Io\Poll: closing a watched stream retires its watcher (GH-22844) +--FILE-- +add(new StreamPollHandle($r), [Io\Poll\Event::Read]); +fwrite($w, "ping"); +fclose($r); +fclose($w); + +echo "events: "; var_dump(count($ctx->wait(0, 0))); + +try { + $watcher->remove(); + echo "remove: no exception\n"; +} catch (\Throwable $e) { + echo "remove: ", $e::class, "\n"; +} + +echo "done\n"; +?> +--EXPECT-- +events: int(0) +remove: Io\Poll\InactiveWatcherException +done diff --git a/main/php_streams.h b/main/php_streams.h index 0dd27b8ee88a..994082390773 100644 --- a/main/php_streams.h +++ b/main/php_streams.h @@ -248,6 +248,8 @@ struct _php_stream { struct _php_stream *enclosing_stream; /* this is a private stream owned by enclosing_stream */ zend_llist *error_list; + + HashTable *poll_watchers; }; /* php_stream */ #define PHP_STREAM_CONTEXT(stream) \ diff --git a/main/streams/streams.c b/main/streams/streams.c index 3d8830d7291f..3f378b4530be 100644 --- a/main/streams/streams.c +++ b/main/streams/streams.c @@ -28,6 +28,7 @@ #include "ext/standard/basic_functions.h" /* for BG(CurrentStatFile) */ #include "ext/standard/php_string.h" /* for php_memnstr, used by php_stream_get_record() */ #include "ext/uri/php_uri.h" +#include "ext/standard/io_poll.h" #include #include #include "php_streams_int.h" @@ -363,6 +364,10 @@ fprintf(stderr, "stream_free: %s:%p[%s] preserve_handle=%d release_cast=%d remov return fclose(stream->stdiocast); } + if (stream->poll_watchers) { + php_io_poll_stream_notify_close(stream); + } + ret = stream->ops->close(stream, preserve_handle ? 0 : 1); stream->abstract = NULL;