From 75ec74826d9f74c46fee3e9db4cf877135f57013 Mon Sep 17 00:00:00 2001 From: Edmond <1571649+EdmondDantes@users.noreply.github.com> Date: Sat, 12 Sep 2026 13:34:17 +0300 Subject: [PATCH] Give a Windows console read a buffer the handle owns uv_read_stop() is not an ownership barrier for a tty: uv__tty_queue_read_line() hands the allocator's address to a ReadConsoleW worker started with QueueUserWorkItem, and the cancellation is an injected VK_RETURN, so the worker returns successfully and writes into memory the cancelled coroutine has already freed. A pipe and a TCP socket allocate in the loop thread once the bytes are there and are not exposed. A tty handle now carries one 8 KB buffer, allocated at the first read and never resized, and the completion copies out of it into the request. libuv releases the handle only through its close callback, which runs once the pending requests have drained. --- CHANGELOG.md | 2 +- docs/286-windows-console-read-cancel.md | 119 ++++++++++++++++++++++++ libuv_reactor.c | 49 +++++++++- libuv_reactor.h | 8 ++ 4 files changed, 176 insertions(+), 2 deletions(-) create mode 100644 docs/286-windows-console-read-cancel.md diff --git a/CHANGELOG.md b/CHANGELOG.md index 55a05eb6..3dcc37ff 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - **A cancelled coroutine left its buffer to a thread-pool worker** (#286). `uv_cancel` does not stop a worker that has started, so a read or a write cancelled mid-flight returned while the worker still named the caller's memory: the next `fclose` freed `stream->readbuf` under a `uv_fs_read` — ASAN reports `WRITE of size ` from `uv__fs_read` — and a cancelled `fwrite` left `uv_fs_write` reading a filter bucket that had already gone, which `strace` shows as `write(...) = -1 EFAULT` (18 of 20 rounds) and which on a recycled block would put another allocation's bytes in the file. A thread-pool read now lands in a buffer of the request's own and the completion copies it across unless the request was abandoned; a thread-pool write copies its payload at submit, before the queue, so that no allocation happens inside a completion callback. Both buffers go with the request, which `libuv_io_req_dispose` frees only after the operation completes, and both come from `malloc` rather than the request allocator: a chunk-sized block per operation would otherwise count twice against `memory_limit`, and freeing it through ZendMM cost an `mmap`/`munmap` pair per read above the 2 MB huge-block threshold. A fire-and-forget write keeps its buffer as before — the reactor already owns it. Nothing in php-src changes. Cost on a release ZTS build, 256 MB read or written, medians of five interleaved runs: 8 KB chunks 1629.7 ms to 1658.8 ms reading and 1471.9 ms to 1505.8 ms writing to `/dev/null`; 1 MB chunks 54.2 ms to 69.8 ms; 4 MB chunks 128.8 ms to 152.6 ms — the residue is the copy itself, which the approach cannot avoid. The footprint is the other half of the price: an in-flight operation now holds a twin of a buffer `memory_limit` already counts, and the twin is outside that limit — a script's real peak is up to twice its accounted one, bounded by the chunk size times the operations in flight. Evidence: `tests/io/099-cancel_during_io.phpt`, which needs the ASAN job to fail: the defect is silent on an ordinary build. - A Windows stream read still hands the caller's buffer to libuv, where a console line read runs on a thread of its own and an overlapped read keeps the address past `uv_read_stop`. The same treatment does not work there: a stream read carries no in-flight marker, so the dispose would free the buffer at cancel time — earlier than today — and the window would widen. Left open in #286. + A Windows console read hands libuv a buffer the handle owns rather than the caller's. `uv_read_stop()` is not an ownership barrier for a tty: `uv__tty_queue_read_line()` calls the allocator and hands the address to a `ReadConsoleW` worker started with `QueueUserWorkItem`, which nothing joins, and the cancellation is an injected `VK_RETURN` — so the worker returns successfully and writes into memory the cancelled coroutine has freed. The other two stream types are not exposed: a pipe waits on a zero-byte `ReadFile` and a TCP socket on a zero-byte read, and both call the allocator only in the loop thread once the bytes are there (libuv 1.49 and 1.51, `src/win/tty.c`, `src/win/pipe.c`, `src/win/tcp.c`); a tty in raw mode allocates in the loop thread too. `async_io_t` therefore carries one 8 KB buffer for tty handles, allocated at the first read and never resized — `uv_tty_line_read_thread` reads at most `MAX_INPUT_BUFFER_LENGTH` = 8192 bytes whatever the allocator offers, so the fixed size never shortens an answer — and the completion copies what arrived into the request's buffer, bounded by what that request asked for. libuv releases the handle only through its close callback, which runs once the pending requests have drained, so the free is late enough; a handle disposed with the reactor already down keeps its buffer, leaked on purpose as the descriptor is in #282. The multishot path, which supplies its own allocator, is untouched. The price is one memcpy per console read and 8 KB per console handle, `malloc` memory outside `memory_limit`. Evidence is `heap-use-after-free` before the change and a clean run after it, WRITE at `uv_utf16_to_wtf8` from `uv_tty_line_read_thread` on memory freed by `php_stream_free`. It takes a console, a libuv built with `/fsanitize=address` — the stock `libuv.lib` is not instrumented, which is why a write from a libuv worker had never shown up in an ASAN run — and a delay injected into that worker; a CI runner has none of the three, so the procedure is written down in `docs/286-windows-console-read-cancel.md` rather than left as a test. - **A closed handle returned its descriptor to the process while a thread-pool worker still named it** (#282). `libuv_io_close()` closed `crt_fd` and woke every parked coroutine with `Stream was closed`, but a `uv_fs_read`/`uv_fs_write` handed to the thread pool goes on running: the next `open()` got the number back and the worker moved that file's offset — `Assertion 'lseek(self->fd, 0, 1) == 0' failed` in `_php_stream_fopen_from_fd` on a debug build, someone else's file read or written on a release one — while the woken coroutine freed the buffer the worker was still reading, which ASAN reports as a heap-use-after-free of the whole write buffer — `092-filter_remove_during_write.phpt` on an ASAN build shows the 3.2 MB read by `uv__fs_write` in a worker thread, and `086-close_during_io.phpt` the 8 KB read buffer written by `uv__fs_read`. Both are clean after the change; the ASAN run of `ext/async/tests` plus php-src `file`, `streams` and `filters` is 1935 tests with one failure, the local network artefact. `async_io_t` now counts the requests the thread pool holds (`fs_in_flight`); a close with a non-zero count leaves the descriptor open and notifies the waiters without an error, so they park again, and the last completion callback closes the descriptor and releases them. php-src hands the descriptor over in `php_stdiop_close()` for a file handle with no `FILE*` of its own, and keeps the stream's read buffer for the same span: `php_stream_free()` freed it in the same call as `ops->close`, so the worker went on writing into freed memory even while the coroutine stayed parked — valgrind counted 20 such writes in one run of `086-close_during_io.phpt` and none after. Measured on `tests/io/086-close_during_io.phpt` with 300 rounds and four processes at once: 5 aborts in 60 runs before, 0 in 260 after, and 0 in 40 on a release build. A cancelled read is the one wake left on the old route: `uv_cancel` cannot preempt a running worker, so the coroutine leaves with the request still in flight and the read buffer is freed by the next `fclose` — #286. diff --git a/docs/286-windows-console-read-cancel.md b/docs/286-windows-console-read-cancel.md new file mode 100644 index 00000000..7ffa0ca4 --- /dev/null +++ b/docs/286-windows-console-read-cancel.md @@ -0,0 +1,119 @@ +# #286 — Windows: a cancelled console read writes into the freed buffer + +**Status:** root cause established from libuv's source and reproduced under ASAN; +fix landed in `libuv_io_alloc_cb` / `io_pipe_read_cb` (a tty handle owns the read +buffer). No automated test — a CI runner has no console. This document is the +procedure that turns the defect red, and the only way to check the fix again. + +## The defect + +`uv_read_stop()` is not an ownership barrier for a tty in line mode. +`uv__tty_queue_read_line()` (`src/win/tty.c`) calls the allocator on the loop +thread and hands the address to `uv_tty_line_read_thread`, started with +`QueueUserWorkItem`; nothing joins that thread. Cancellation is not a stop but an +injected keystroke: `uv__cancel_read_console()` writes a `VK_RETURN` record into +the console input, so `ReadConsoleW` returns **successfully**, with one or two +characters, and the worker writes them through `uv_utf16_to_wtf8` into the buffer +the allocator gave it. libuv then drops the result, because the request is marked +`UV_HANDLE_CANCELLATION_PENDING` — `read_cb` is never called for a cancelled line +read, which is why a rendezvous on the read completion cannot work. + +While the reactor handed libuv `req->base.buf`, that address was the PHP stream's +`readbuf`, freed by `php_stream_free()` as soon as the cancelled coroutine let the +stream go. + +Not exposed, and why: + +- **Pipe** — `uv__pipe_read_data()` (`src/win/pipe.c`) calls the allocator on the + loop thread once the zero-byte `ReadFile` has signalled, and reads into it in + the same callback. +- **TCP** — `uv__tcp_queue_read()` always arms a zero-byte read in 1.49 and 1.51; + `uv__process_tcp_read_req()` allocates inside the loop-thread `WSARecv` loop. +- **Tty in raw mode** — `uv__tty_queue_read_raw()` registers a wait and allocates + in the loop thread; no worker sees the address. + +## Why no ASAN run had ever caught it + +`E:\php\deps\lib\libuv.lib` is a prebuilt static library and is **not** compiled +with `/fsanitize=address`. ASAN reports a bad access only from instrumented code, +and the write happens inside libuv, in a hand-written loop rather than an +intercepted `memcpy`. Every "clean under Windows ASAN" claim about a write from a +libuv worker thread — this defect included — was therefore vacuous. Rebuild libuv +with the sanitizer before believing such a run. + +## Reproducing it + +1. Get a libuv source tree matching what the build links (the version in + `deps/include/libuv/uv/version.h`; the paths below are 1.51, whose `tty.c`, + `pipe.c` and `tcp.c` are the same as 1.49 on these routes). + +2. Patch `src/win/tty.c`, in `uv_tty_line_read_thread`, between the `ReadConsoleW` + call and the `uv_utf16_to_wtf8` that follows it: + + ```c + Sleep(200); /* widen the race: let the free happen first */ + ``` + + The delay is the whole trick. Without it the worker wins by about a + millisecond and the write lands in memory that is still allocated, so the run + is clean whatever the code does. Load on the machine does not help, because it + slows both sides of the race equally. + +3. Build it with the sanitizer and a matching runtime, and install it over the + prebuilt one (keep a copy of the original library and headers): + + ``` + cmake -B build -G Ninja -DCMAKE_BUILD_TYPE=Release -DBUILD_TESTING=OFF \ + -DLIBUV_BUILD_SHARED=OFF -DCMAKE_MSVC_RUNTIME_LIBRARY=MultiThreadedDLL \ + -DCMAKE_C_FLAGS="/MD /Zi /fsanitize=address" + cmake --build build + copy build\libuv.lib E:\php\deps\lib\libuv.lib + ``` + +4. Build php-src with `--enable-sanitizer --enable-zts --enable-async` and run + the script below **in a real console window** — a test runner's stdin is a + pipe, and `fopen('CONIN$')` answers `false` even from a console, so the script + has to be started so that it owns a console of its own (`Start-Process`, or + `start` from `cmd`). Redirect stdout to a file if the output is in the way; + stdin stays the console. + + ```php + fread($h, 4096)); + async\spawn(static function () use ($reader, $h) { + async\delay(50); + $reader->cancel(); + fclose($h); + $junk = []; + for ($k = 0; $k < 300; $k++) { $junk[] = str_repeat('x', 8192); } + }); + try { async\await($reader); } catch (Throwable $e) { } + ``` + + Run it with `USE_ZEND_ALLOC=0`, so that the stream buffer comes from the + system allocator and ASAN can see the free. + +**One round per process.** `uv__read_console_status` is a single global for the +whole process, not per handle, so two console workers alive at once make a later +cancellation see `COMPLETED` and skip the injection; with `Sleep(200)` in place +the second round then blocks forever. That is an artefact of the probe meeting a +libuv limitation, not of the reactor. + +## What red and green look like + +Without the fix: + +``` +==17108==ERROR: AddressSanitizer: heap-use-after-free on address 0x1264f0cab500 +WRITE of size 1 at 0x1264f0cab500 thread T-1 + #0 uv_utf16_to_wtf8 src\idna.c:504 + #1 uv_tty_line_read_thread src\win\tty.c:576 +freed by thread T0 here: + #3 php_stream_free main\streams\streams.c:473 + #4 zif_fclose ext\standard\file.c:779 +``` + +With it, the same run prints nothing: the worker writes into `io->tty_read_buf`, +which the handle keeps until libuv's close callback, and the completion has +already copied out of it whatever the request asked for. diff --git a/libuv_reactor.c b/libuv_reactor.c index 7508b7bb..adce3f1d 100644 --- a/libuv_reactor.c +++ b/libuv_reactor.c @@ -4724,6 +4724,15 @@ static bool libuv_io_event_dispose(zend_async_event_t *event) zend_async_callbacks_free(event); +#ifdef PHP_WIN32 + /* The close callback runs after the worker, so the buffer is free to go. + * With the reactor down that callback never ran: leak it, as #282 leaks + * the descriptor. */ + if (io->tty_read_buf != NULL && EXPECTED(ASYNC_G(reactor_started))) { + pefree(io->tty_read_buf, 1); + } +#endif + pefree(io, 0); return true; @@ -4858,6 +4867,12 @@ static zend_always_inline void async_uv_read_buf_set(uv_buf_t *buf, char *base, buf->base = base; } +#ifdef PHP_WIN32 +/* MAX_INPUT_BUFFER_LENGTH: a console never answers with more, and + * uv_utf16_to_wtf8's trailing NUL stays inside the block. */ +#define ASYNC_IO_TTY_READ_BUF_SIZE 8192 +#endif + static void libuv_io_alloc_cb(uv_handle_t *handle, size_t suggested_size, uv_buf_t *output) { async_io_t *io = (async_io_t *) handle->data; @@ -4888,6 +4903,22 @@ static void libuv_io_alloc_cb(uv_handle_t *handle, size_t suggested_size, uv_buf return; } +#ifdef PHP_WIN32 + /* A console worker outlives the request, so it gets the handle's buffer + * and the completion copies out of it (#286). */ + if (io->base.type == ZEND_ASYNC_IO_TYPE_TTY) { + if (io->tty_read_buf == NULL) { + io->tty_read_buf = pemalloc(ASYNC_IO_TTY_READ_BUF_SIZE, 1); + } + + const size_t length = + req->max_size < ASYNC_IO_TTY_READ_BUF_SIZE ? req->max_size : ASYNC_IO_TTY_READ_BUF_SIZE; + + async_uv_read_buf_set(output, io->tty_read_buf, length); + return; + } +#endif + async_uv_read_buf_set(output, req->base.buf, req->max_size); } @@ -4915,8 +4946,24 @@ static void io_pipe_read_cb(uv_stream_t *pipe_stream, ssize_t bytes_read, const return; } + ssize_t delivered = bytes_read; + +#ifdef PHP_WIN32 + /* Bytes in the handle's buffer belong to whichever request is current, up + * to what it asked for; a request with no buffer takes none. */ + if (bytes_read > 0 && io->tty_read_buf != NULL && buffer->base == io->tty_read_buf) { + delivered = EXPECTED(req->base.buf != NULL) + ? (ssize_t) ((size_t) bytes_read < req->max_size ? (size_t) bytes_read : req->max_size) + : 0; + + if (delivered > 0) { + memcpy(req->base.buf, io->tty_read_buf, (size_t) delivered); + } + } +#endif + if (bytes_read > 0) { - req->base.transferred = bytes_read; + req->base.transferred = delivered; } else if (bytes_read == UV_EOF) { req->base.transferred = 0; io->base.state |= ZEND_ASYNC_IO_EOF; diff --git a/libuv_reactor.h b/libuv_reactor.h index f71bdf98..b5cd6181 100644 --- a/libuv_reactor.h +++ b/libuv_reactor.h @@ -199,6 +199,14 @@ struct _async_io_t * caller's buffer, and the close waits. */ unsigned fs_in_flight; +#ifdef PHP_WIN32 + /* Read buffer of a console handle, freed only by its close callback: the + * ReadConsoleW worker keeps writing after the read is stopped, so it cannot + * be the caller's (#286, docs/286-windows-console-read-cancel.md). Fixed + * size, one per handle — libuv serialises the reads of a handle. */ + char *tty_read_buf; +#endif + union { uv_stream_t stream;