[newlib-cygwin] Cygwin: pty: do not leak nat handles when adopting the pcon's in open_setup()

Takashi Yano via Cygwin-cvs <[email protected]> Tue, 30 Jun 2026 08:34:43 +0000 (GMT)
Newsgroups gmane.os.cygwin.cvs
Message-ID <[email protected]>
https://sourceware.org/git/gitweb.cgi?p=3Dnewlib-cygwin.git;h=3D60a88896dce=
0e1c080172cae0118f920f4c28095

commit 60a88896dce0e1c080172cae0118f920f4c28095
Author: Johannes Schindelin <[email protected]>
Date:   Thu Jun 25 13:41:43 2026 +0200

    Cygwin: pty: do not leak nat handles when adopting the pcon's in open_s=
etup()
   =20
    When a Cygwin process opens a pty slave whose pseudo console is already
    active, open() has just installed duplicates of the cyg master-side
    pipe ends into io_handle_nat and output_handle_nat. The pcon adoption
    added in "Cygwin: pty: Fixup pty state after a cygwin app exits"
    overwrites those two slots via &get_handle_nat() / &get_output_handle_n=
at()
    without closing them first, so two handles leak on every pcon-backed
    grandchild open. It also hands the result of OpenProcess() straight to
    DuplicateHandle() without a NULL check, so if the nat-pipe owner has
    already exited both duplications fail and leave the nat slots NULL,
    which then breaks the slave's input routing.
   =20
    Close the old slots before replacing them, skip the replacement
    entirely when OpenProcess() returns NULL so we degrade to the handles
    open() installed, and make the pair transactional so a partial success
    cannot leave one original slot and one pcon slot.
   =20
    Fixes: b34394d456b6 ("Cygwin: pty: Fixup pty state after a cygwin app e=
xits")
    Assisted-by: Opus 4.8
    Signed-off-by: Johannes Schindelin <[email protected]>
    Reviewed-by: Takashi Yano <[email protected]>

Diff:
---
 winsup/cygwin/fhandler/pty.cc | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc
index 35e320507..847e5a082 100644
--- a/winsup/cygwin/fhandler/pty.cc
+++ b/winsup/cygwin/fhandler/pty.cc
@@ -1212,13 +1212,33 @@ fhandler_pty_slave::open_setup (int flags)
     {
       HANDLE pcon_owner =3D OpenProcess (PROCESS_DUP_HANDLE, FALSE,
 				       get_ttyp ()->nat_pipe_owner_pid);
-      DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_in,
-		       GetCurrentProcess (), &get_handle_nat (),
-		       0, TRUE, DUPLICATE_SAME_ACCESS);
-      DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
-		       GetCurrentProcess (), &get_output_handle_nat (),
-		       0, TRUE, DUPLICATE_SAME_ACCESS);
-      CloseHandle (pcon_owner);
+      if (pcon_owner)
+	{
+	  HANDLE new_in =3D NULL, new_out =3D NULL;
+	  bool ok_in =3D DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_in,
+				       GetCurrentProcess (), &new_in,
+				       0, TRUE, DUPLICATE_SAME_ACCESS);
+	  bool ok_out =3D DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
+				        GetCurrentProcess (), &new_out,
+				        0, TRUE, DUPLICATE_SAME_ACCESS);
+	  if (ok_in && ok_out)
+	    {
+	      /* Close the cyg master-side handles open() installed before
+		 replacing them, so they do not leak. */
+	      CloseHandle (get_handle_nat ());
+	      CloseHandle (get_output_handle_nat ());
+	      set_handle_nat (new_in);
+	      set_output_handle_nat (new_out);
+	    }
+	  else
+	    {
+	      if (new_in)
+		CloseHandle (new_in);
+	      if (new_out)
+		CloseHandle (new_out);
+	    }
+	  CloseHandle (pcon_owner);
+	}
     }
=20
   set_flags ((flags & ~O_TEXT) | O_BINARY);