Re: [PATCH] Cygwin: pty: detect pcon-backed pty for non-Cygwin-spawned children
Johannes Schindelin <[email protected]> Sat, 27 Jun 2026 10:18:27 +0200 (CEST)
| Newsgroups | gmane.os.cygwin.patches |
|---|---|
| Message-ID | <[email protected]> |
Hi Takashi & Mark, On Wed, 24 Jun 2026, Takashi Yano wrote: > On Thu, 30 Apr 2026 15:04:04 +0000 > "Johannes Schindelin via GitGitGadget" <[email protected]> wrote: > > From: Johannes Schindelin <[email protected]> > > > > When a Cygwin process (e.g. `bash` under MinTTY) spawns a native > > Win32 child (e.g. `git.exe`) with pseudo console support enabled, > > the child gets a pseudo console that bridges the pty. If that native > > child then spawns a Cygwin grandchild (e.g. `vim`, `less`), the > > grandchild inherits the pseudo console's console handles. In > > `init_std_file_from_handle()`, the grandchild's msys2-runtime sees > > `GetConsoleScreenBufferInfo()` succeed on those handles and, with > > no valid `ctty` set, falls back to `FH_CONSOLE` and gives the > > process `cons0` instead of connecting to the pty. > > > [...] > > Pushed to master branch with my fixup patches. I had a buffer-grow follow-up to this patch sitting in https://github.com/git-for-windows/msys2-runtime/pull/131/commits/77e01abd83836b4b7488328ca899aea3a8e4ffbe that I should have sent before you picked up the original and pushed it to master; sorry for the delay. -- snip -- From 77e01abd83836b4b7488328ca899aea3a8e4ffbe Mon Sep 17 00:00:00 2001 From: Johannes Schindelin <[email protected]> Date: Fri, 29 May 2026 19:07:26 +0200 Subject: [PATCH] Cygwin: pty: grow GetConsoleProcessList buffer in find_pcon_pty() find_pcon_pty() was passing a fixed 128-DWORD stack array to GetConsoleProcessList(). If the calling Cygwin process happens to be attached to a console with more than 128 processes, the Win32 function returns the required size and the buffer contents are undefined; the existing if-zero check did not catch that case, so the subsequent loop walked uninitialised data and could either miss the candidate pty or, worse, match against junk PIDs and return the wrong tty index. Adopt the buffer-too-small dance from fhandler_termios::get_console_process_id() in winsup/cygwin/fhandler/termios.cc, which already had to solve this problem and which also notes that the new condrv does not accept oversized first-call buffers (https://github.com/microsoft/terminal/issues/18264#issuecomment-2515448548). The buffer comes from tmp_pathbuf so the same NT_MAX_PATH cap (currently 1024 DWORDs, i.e. 4096 processes) applies; we bail out with -1 if even that is not enough rather than allocate unbounded memory or guess. Bumping the start-with size from 1 would defeat the condrv work-around mentioned above, so we keep the same one-element initial probe as termios.cc and let the loop grow. Suggested-by: Takashi Yano <[email protected]> Assisted-by: Opus 4.7 Signed-off-by: Johannes Schindelin <[email protected]> (cherry picked from commit b65e1544d45567f0033c57a0aa1543c5e654950a) --- winsup/cygwin/tty.cc | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/winsup/cygwin/tty.cc b/winsup/cygwin/tty.cc index 5cce05de34..9bc2a084fb 100644 --- a/winsup/cygwin/tty.cc +++ b/winsup/cygwin/tty.cc @@ -19,6 +19,7 @@ details. */ #include "cygheap.h" #include "pinfo.h" #include "shared_info.h" +#include "tls_pbuf.h" HANDLE NO_COPY tty_list::mutex = NULL; @@ -135,7 +136,9 @@ tty_list::init () int tty_list::find_pcon_pty () { - DWORD pids[128]; + tmp_pathbuf tp; + DWORD *pids = (DWORD *) tp.c_get (); + const DWORD buf_size = NT_MAX_PATH / sizeof (DWORD); DWORD count = 0; bool got_pids = false; @@ -144,10 +147,20 @@ tty_list::find_pcon_pty () if (!ttys[i].has_active_pcon ()) continue; - /* Fetch the console process list lazily, only on first candidate. */ + /* Fetch the console process list lazily, only on first candidate. + The buffer-too-large dance mirrors the one in termios.cc's + get_console_process_id() and works around new condrv's dislike + of oversized first-call buffers, see + https://github.com/microsoft/terminal/issues/18264#issuecomment-2515448548 */ if (!got_pids) { - count = GetConsoleProcessList (pids, 128); + DWORD buf_size1 = 1; + while ((count = GetConsoleProcessList (pids, buf_size1)) > buf_size1) + { + if (count > buf_size) + return -1; + buf_size1 = count; + } if (!count) return -1; got_pids = true; -- snap -- Ciao, Johannes