Skip to content

Commit d02c172

Browse files
committed
Fix GH-22844: StreamPollHandle use-after-free after fclose()
StreamPollHandle cached the raw php_stream* but referenced only the resource container, not the stream, so fclose() freed the stream while the handle kept a dangling pointer that isValid(), getStream() and get_fd dereferenced. Re-derive the stream from the held resource each time. A watcher likewise re-derived its registration fd from that stream in remove(), so removing a watcher after its stream closed leaked the Poll-backend entry and could be re-hit through a reused fd in wait(); cache the fd at add() and use it for removal. Fixes GH-22844
1 parent f2b6b7f commit d02c172

7 files changed

Lines changed: 117 additions & 20 deletions

File tree

NEWS

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,6 +51,10 @@ PHP NEWS
5151
TCP_USER_TIMEOUT, and SO_LINGER options. (Weilin Du)
5252
. Fixed various memory related issues in ext/sockets. (David Carlier)
5353

54+
- Standard:
55+
. Fixed bug GH-22844 (Use-after-free in StreamPollHandle after the
56+
underlying stream is closed). (iliaal)
57+
5458
- Streams:
5559
. Added a new IO copy API used by php_stream_copy_to_stream_ex() that
5660
leverages platform primitives (sendfile, splice, copy_file_range,

ext/standard/io_poll.c

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,7 @@ typedef struct php_io_poll_watcher_object {
5454
zval data;
5555
bool active;
5656
php_io_poll_context_object *context; /* Back reference to Context object */
57+
php_socket_t fd;
5758
zend_object std;
5859
} php_io_poll_watcher_object;
5960

@@ -66,7 +67,6 @@ struct php_io_poll_context_object {
6667

6768
/* Stream poll handle specific data */
6869
typedef struct php_stream_poll_handle_data {
69-
php_stream *stream;
7070
zend_resource *res;
7171
} php_stream_poll_handle_data;
7272

@@ -179,16 +179,26 @@ static const char *php_io_poll_backend_type_to_name(php_poll_backend_type type)
179179

180180
/* Stream Poll Handle Implementation */
181181

182+
static zend_always_inline php_stream *php_stream_poll_handle_get_stream(php_stream_poll_handle_data *data)
183+
{
184+
if (!data || !data->res
185+
|| (data->res->type != php_file_le_stream()
186+
&& data->res->type != php_file_le_pstream())) {
187+
return NULL;
188+
}
189+
return (php_stream *) data->res->ptr;
190+
}
191+
182192
static php_socket_t php_stream_poll_handle_get_fd(php_poll_handle_object *handle)
183193
{
184-
php_stream_poll_handle_data *data = handle->handle_data;
194+
php_stream *stream = php_stream_poll_handle_get_stream(handle->handle_data);
185195
php_socket_t fd;
186196

187-
if (!data || !data->stream) {
197+
if (!stream) {
188198
return SOCK_ERR;
189199
}
190200

191-
if (php_stream_cast(data->stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL,
201+
if (php_stream_cast(stream, PHP_STREAM_AS_FD_FOR_SELECT | PHP_STREAM_CAST_INTERNAL,
192202
(void *) &fd, 1)
193203
!= SUCCESS
194204
|| fd == -1) {
@@ -200,8 +210,8 @@ static php_socket_t php_stream_poll_handle_get_fd(php_poll_handle_object *handle
200210

201211
static int php_stream_poll_handle_is_valid(php_poll_handle_object *handle)
202212
{
203-
php_stream_poll_handle_data *data = handle->handle_data;
204-
return data && data->stream && !php_stream_eof(data->stream);
213+
php_stream *stream = php_stream_poll_handle_get_stream(handle->handle_data);
214+
return stream && !php_stream_eof(stream);
205215
}
206216

207217
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)
254264
intern->triggered_events = 0;
255265
intern->active = false;
256266
intern->context = NULL;
267+
intern->fd = SOCK_ERR;
257268
ZVAL_NULL(&intern->data);
258269

259270
return &intern->std;
@@ -453,7 +464,6 @@ PHP_METHOD(StreamPollHandle, __construct)
453464

454465
/* Set up stream-specific data */
455466
php_stream_poll_handle_data *data = emalloc(sizeof(php_stream_poll_handle_data));
456-
data->stream = stream;
457467
data->res = stream->res;
458468
intern->handle_data = data;
459469

@@ -466,14 +476,14 @@ PHP_METHOD(StreamPollHandle, getStream)
466476
ZEND_PARSE_PARAMETERS_NONE();
467477

468478
php_poll_handle_object *intern = PHP_POLL_HANDLE_OBJ_FROM_ZV(getThis());
469-
php_stream_poll_handle_data *data = intern->handle_data;
479+
php_stream *stream = php_stream_poll_handle_get_stream(intern->handle_data);
470480

471-
if (!data || !data->stream) {
481+
if (!stream) {
472482
RETURN_NULL();
473483
}
474484

475-
GC_ADDREF(data->stream->res);
476-
php_stream_to_zval(data->stream, return_value);
485+
GC_ADDREF(stream->res);
486+
php_stream_to_zval(stream, return_value);
477487
}
478488

479489
PHP_METHOD(StreamPollHandle, isValid)
@@ -645,13 +655,13 @@ PHP_METHOD(Io_Poll_Watcher, remove)
645655
php_poll_ctx *poll_ctx = context->ctx;
646656
HashTable *watchers = context->watchers;
647657
zend_ulong hash_key = php_io_poll_compute_ptr_key(intern->handle);
648-
php_socket_t fd = php_poll_handle_get_fd(intern->handle);
649-
if (fd != SOCK_ERR) {
650-
php_poll_remove(poll_ctx, (int) fd);
658+
if (intern->fd != SOCK_ERR) {
659+
php_poll_remove(poll_ctx, (int) intern->fd);
651660
}
652661

653662
intern->active = false;
654663
intern->context = NULL;
664+
intern->fd = SOCK_ERR;
655665

656666
if (watchers) {
657667
zend_hash_index_del(watchers, hash_key);
@@ -770,6 +780,7 @@ PHP_METHOD(Io_Poll_Context, add)
770780

771781
watcher->active = true;
772782
watcher->context = intern;
783+
watcher->fd = fd;
773784
}
774785

775786
PHP_METHOD(Io_Poll_Context, wait)

ext/standard/io_poll.stub.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ final class StreamPollHandle implements Io\Poll\Handle
159159
/** @param resource $stream */
160160
public function __construct($stream) {}
161161

162-
/** @return resource */
162+
/** @return resource|null */
163163
public function getStream() {}
164164

165165
public function isValid(): bool {}

ext/standard/io_poll_arginfo.h

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

ext/standard/io_poll_decl.h

Lines changed: 4 additions & 4 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
--TEST--
2+
GH-22844 (StreamPollHandle accessors are safe after the stream is closed)
3+
--FILE--
4+
<?php
5+
require_once __DIR__ . '/poll.inc';
6+
7+
[$r, $w] = pt_new_socket_pair();
8+
$context = pt_new_stream_poll();
9+
$handle = new StreamPollHandle($r);
10+
11+
echo "before close:\n";
12+
echo "isValid: "; var_dump($handle->isValid());
13+
echo "getStream is resource: "; var_dump(is_resource($handle->getStream()));
14+
15+
fclose($r);
16+
fclose($w);
17+
18+
echo "after close:\n";
19+
echo "isValid: "; var_dump($handle->isValid());
20+
echo "getStream: "; var_dump($handle->getStream());
21+
22+
try {
23+
$context->add($handle, [Io\Poll\Event::Read]);
24+
echo "add: no exception\n";
25+
} catch (\Throwable $e) {
26+
echo "add: ", $e::class, ": ", $e->getMessage(), "\n";
27+
}
28+
29+
echo "done\n";
30+
?>
31+
--EXPECT--
32+
before close:
33+
isValid: bool(true)
34+
getStream is resource: bool(true)
35+
after close:
36+
isValid: bool(false)
37+
getStream: NULL
38+
add: Io\Poll\InvalidHandleException: Invalid handle for polling
39+
done
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
--TEST--
2+
Io\Poll: removing a watcher after its stream is closed unregisters the backend fd (GH-22844)
3+
--SKIPIF--
4+
<?php
5+
if (!Io\Poll\Backend::Poll->isAvailable()) {
6+
die("skip Poll backend not available\n");
7+
}
8+
?>
9+
--FILE--
10+
<?php
11+
require_once __DIR__ . '/poll.inc';
12+
13+
// The Poll backend keeps a userspace fd->watcher map. If remove() cannot
14+
// unregister the fd (because the stream was already closed) a stale entry is
15+
// left behind and re-hit once that fd number is reused. Auto (epoll) drops
16+
// closed fds in the kernel, so force Poll to exercise the registration path.
17+
$ctx = new Io\Poll\Context(Io\Poll\Backend::Poll);
18+
[$r, $w] = pt_new_socket_pair();
19+
20+
$watcher = $ctx->add(new StreamPollHandle($r), [Io\Poll\Event::Read]);
21+
fclose($r);
22+
fclose($w);
23+
24+
// remove() must still unregister the fd even though the stream is gone.
25+
$watcher->remove();
26+
unset($watcher);
27+
gc_collect_cycles();
28+
29+
// Reclaim the freed fd numbers with fresh readable sockets. A dangling
30+
// registration would map one of these to the freed watcher inside wait().
31+
$keep = [];
32+
for ($i = 0; $i < 6; $i++) {
33+
[$a, $b] = pt_new_socket_pair();
34+
fwrite($b, "ping");
35+
$keep[] = [$a, $b];
36+
}
37+
38+
echo "events: "; var_dump(count($ctx->wait(0, 0)));
39+
echo "done\n";
40+
?>
41+
--EXPECT--
42+
events: int(0)
43+
done

0 commit comments

Comments
 (0)