Fix GH-22844: StreamPollHandle use-after-free after fclose()

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)
+
 - Streams:
   . Added a new IO copy API used by php_stream_copy_to_stream_ex() that
     leverages platform primitives (sendfile, splice, copy_file_range,

ext/standard/io_poll.c

@@ -54,6 +54,7 @@ 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;
 	zend_object std;
 } php_io_poll_watcher_object;
 
@@ -66,7 +67,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 +179,26 @@ 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
+			|| (data->res->type != php_file_le_stream()
+				&& data->res->type != php_file_le_pstream())) {
+		return NULL;
+	}
+	return (php_stream *) data->res->ptr;
+}
+
 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,7 @@ 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;
 	ZVAL_NULL(&intern->data);
 
 	return &intern->std;
@@ -453,7 +464,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 +476,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 +655,13 @@ 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);
 	}
 
 	intern->active = false;
 	intern->context = NULL;
+	intern->fd = SOCK_ERR;
 
 	if (watchers) {
 		zend_hash_index_del(watchers, hash_key);
@@ -770,6 +780,7 @@ PHP_METHOD(Io_Poll_Context, add)
 
 	watcher->active = true;
 	watcher->context = intern;
+	watcher->fd = fd;
 }
 
 PHP_METHOD(Io_Poll_Context, wait)

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 {}

ext/standard/tests/poll/gh22844.phpt

@@ -0,0 +1,39 @@
+--TEST--
+GH-22844 (StreamPollHandle accessors are safe after the stream is closed)
+--FILE--
+<?php
+require_once __DIR__ . '/poll.inc';
+
+[$r, $w] = pt_new_socket_pair();
+$context = pt_new_stream_poll();
+$handle = new StreamPollHandle($r);
+
+echo "before close:\n";
+echo "isValid: "; var_dump($handle->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

ext/standard/tests/poll/poll_stream_handle_remove_after_close.phpt

@@ -0,0 +1,43 @@
+--TEST--
+Io\Poll: removing a watcher after its stream is closed unregisters the backend fd (GH-22844)
+--SKIPIF--
+<?php
+if (!Io\Poll\Backend::Poll->isAvailable()) {
+    die("skip Poll backend not available\n");
+}
+?>
+--FILE--
+<?php
+require_once __DIR__ . '/poll.inc';
+
+// The Poll backend keeps a userspace fd->watcher map. If remove() cannot
+// unregister the fd (because the stream was already closed) a stale entry is
+// left behind and re-hit once that fd number is reused. Auto (epoll) drops
+// closed fds in the kernel, so force Poll to exercise the registration path.
+$ctx = new Io\Poll\Context(Io\Poll\Backend::Poll);
+[$r, $w] = pt_new_socket_pair();
+
+$watcher = $ctx->add(new StreamPollHandle($r), [Io\Poll\Event::Read]);
+fclose($r);
+fclose($w);
+
+// remove() must still unregister the fd even though the stream is gone.
+$watcher->remove();
+unset($watcher);
+gc_collect_cycles();
+
+// Reclaim the freed fd numbers with fresh readable sockets. A dangling
+// registration would map one of these to the freed watcher inside wait().
+$keep = [];
+for ($i = 0; $i < 6; $i++) {
+    [$a, $b] = pt_new_socket_pair();
+    fwrite($b, "ping");
+    $keep[] = [$a, $b];
+}
+
+echo "events: "; var_dump(count($ctx->wait(0, 0)));
+echo "done\n";
+?>
+--EXPECT--
+events: int(0)
+done