Skip to content

Commit 60a8889

Browse files
dschotyan0
authored andcommitted
Cygwin: pty: do not leak nat handles when adopting the pcon's in open_setup()
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_nat() 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. 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. Fixes: b34394d ("Cygwin: pty: Fixup pty state after a cygwin app exits") Assisted-by: Opus 4.8 Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> Reviewed-by: Takashi Yano <takashi.yano@nifty.ne.jp>
1 parent 10140be commit 60a8889

1 file changed

Lines changed: 27 additions & 7 deletions

File tree

winsup/cygwin/fhandler/pty.cc

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1212,13 +1212,33 @@ fhandler_pty_slave::open_setup (int flags)
12121212
{
12131213
HANDLE pcon_owner = OpenProcess (PROCESS_DUP_HANDLE, FALSE,
12141214
get_ttyp ()->nat_pipe_owner_pid);
1215-
DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_in,
1216-
GetCurrentProcess (), &get_handle_nat (),
1217-
0, TRUE, DUPLICATE_SAME_ACCESS);
1218-
DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
1219-
GetCurrentProcess (), &get_output_handle_nat (),
1220-
0, TRUE, DUPLICATE_SAME_ACCESS);
1221-
CloseHandle (pcon_owner);
1215+
if (pcon_owner)
1216+
{
1217+
HANDLE new_in = NULL, new_out = NULL;
1218+
bool ok_in = DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_in,
1219+
GetCurrentProcess (), &new_in,
1220+
0, TRUE, DUPLICATE_SAME_ACCESS);
1221+
bool ok_out = DuplicateHandle (pcon_owner, get_ttyp ()->h_pcon_out,
1222+
GetCurrentProcess (), &new_out,
1223+
0, TRUE, DUPLICATE_SAME_ACCESS);
1224+
if (ok_in && ok_out)
1225+
{
1226+
/* Close the cyg master-side handles open() installed before
1227+
replacing them, so they do not leak. */
1228+
CloseHandle (get_handle_nat ());
1229+
CloseHandle (get_output_handle_nat ());
1230+
set_handle_nat (new_in);
1231+
set_output_handle_nat (new_out);
1232+
}
1233+
else
1234+
{
1235+
if (new_in)
1236+
CloseHandle (new_in);
1237+
if (new_out)
1238+
CloseHandle (new_out);
1239+
}
1240+
CloseHandle (pcon_owner);
1241+
}
12221242
}
12231243

12241244
set_flags ((flags & ~O_TEXT) | O_BINARY);

0 commit comments

Comments
 (0)