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