[newlib-cygwin] Cygwin: pty: Guard to_be_read_from_nat_pipe() by pipe_sw_mutex
Takashi Yano via Cygwin-cvs <[email protected]> Sun, 29 Mar 2026 00:44:10 +0000 (GMT)
| Newsgroups | gmane.os.cygwin.cvs |
|---|---|
| Message-ID | <[email protected]> |
https://sourceware.org/git/gitweb.cgi?p=3Dnewlib-cygwin.git;h=3D2f705b879fa= c45e5cd761fad8dd7de6b388aee2a commit 2f705b879fac45e5cd761fad8dd7de6b388aee2a Author: Takashi Yano <[email protected]> Date: Tue Mar 17 13:11:44 2026 +0900 Cygwin: pty: Guard to_be_read_from_nat_pipe() by pipe_sw_mutex =20 `to_be_read_from_nat_pipe()` reads several shared-memory fields (`switch_to_nat_pipe`, `pcon_activated`, `pty_input_state`) to decide whether keystrokes should go to the nat pipe. It is called from `master::write()` on every keystroke. Without synchronization, the slave can be in the middle of a pipe switch (changing these fields in `setup_for_non_cygwin_app()`, `cleanup_for_non_cygwin_app()`, or `setpgid_aux()`) while the master reads a half-updated snapshot, making an inconsistent routing decision that sends keystrokes to the wrong pipe. =20 Guard `to_be_read_from_nat_pipe()` with `pipe_sw_mutex` so it always reads a consistent state. The spin-wait at entry handles the pseudo console initialization case: when `pipe_sw_mutex` is held by the slave during `setup_pseudoconsole()` and `pcon_start` is set, the function returns false immediately, routing keystrokes to the cyg pipe through `line_edit()` where the CSI6n response handler expects them. =20 Acquiring `pipe_sw_mutex` inside `to_be_read_from_nat_pipe()` creates a lock ordering constraint: `master::write()` holds `input_mutex` before calling `to_be_read_from_nat_pipe()`, so the master's lock order is `input_mutex` then `pipe_sw_mutex`. Previously, `cleanup_for_non_cygwin_app()` and `setpgid_aux()` acquired `pipe_sw_mutex` first and then `input_mutex` (for `transfer_input()`), which is the reverse order and would deadlock. Restructure both functions to release `pipe_sw_mutex` before acquiring `input_mutex`, maintaining a consistent lock order throughout. =20 Fixes: bb4285206207 ("Cygwin: pty: Implement new pseudo console support= .") Signed-off-by: Takashi Yano <[email protected]> Reviewed-by: Johannes Schindelin <[email protected]> Diff: --- winsup/cygwin/fhandler/pty.cc | 48 ++++++++++++++++++++++++++++++---------= ---- 1 file changed, 34 insertions(+), 14 deletions(-) diff --git a/winsup/cygwin/fhandler/pty.cc b/winsup/cygwin/fhandler/pty.cc index ac5f67bc4..18285b874 100644 --- a/winsup/cygwin/fhandler/pty.cc +++ b/winsup/cygwin/fhandler/pty.cc @@ -1284,22 +1284,42 @@ fhandler_pty_slave::mask_switch_to_nat_pipe (bool m= ask, bool xfer) bool fhandler_pty_common::to_be_read_from_nat_pipe (void) { + /* If the slave is in setup_pseudoconsole(), pipe_sw_mutex cannot + be acquired because the slave has it. In this case pcon_start + will be asserted. During pcon_start, other input than response + to CSI6n should be go to cyg-pipe. So, wait for pcon_start and + return false. */ + while (WaitForSingleObject (pipe_sw_mutex, 0) =3D=3D WAIT_TIMEOUT) + if (get_ttyp ()->pcon_start || get_ttyp ()->pcon_start_pid) + return false; + else + yield (); + + bool ret =3D false; if (!get_ttyp ()->switch_to_nat_pipe) - return false; + goto out; =20 - char name[MAX_PATH]; - shared_name (name, TTY_SLAVE_READING, get_minor ()); - HANDLE masked =3D OpenEvent (READ_CONTROL, FALSE, name); - CloseHandle (masked); + { + char name[MAX_PATH]; + shared_name (name, TTY_SLAVE_READING, get_minor ()); + HANDLE masked =3D OpenEvent (READ_CONTROL, FALSE, name); + CloseHandle (masked); =20 - if (masked) /* The foreground process is cygwin process */ - return false; + if (masked) /* The foreground process is cygwin process */ + goto out; + } =20 if (!pinfo (get_ttyp ()->getpgid ())) /* GDB may set invalid process group for non-cygwin process. */ - return true; + { + ret =3D true; + goto out; + } =20 - return get_ttyp ()->nat_fg (get_ttyp ()->getpgid ()); + ret =3D get_ttyp ()->nat_fg (get_ttyp ()->getpgid ()); +out: + ReleaseMutex (pipe_sw_mutex); + return ret; } =20 void @@ -3941,7 +3961,6 @@ fhandler_pty_slave::term_has_pcon_cap (const WCHAR *e= nv) goto maybe_dumb; =20 /* Check if terminal has CSI6n */ - WaitForSingleObject (pipe_sw_mutex, INFINITE); WaitForSingleObject (input_mutex, mutex_timeout); /* Set pcon_activated and pcon_start so that the response will sent to io_handle_nat rather than io_handle. */ @@ -3977,7 +3996,6 @@ fhandler_pty_slave::term_has_pcon_cap (const WCHAR *e= nv) while (len); get_ttyp ()->pcon_activated =3D false; get_ttyp ()->nat_pipe_owner_pid =3D 0; - ReleaseMutex (pipe_sw_mutex); if (len =3D=3D 0) goto not_has_csi6n; =20 @@ -3993,7 +4011,6 @@ not_has_csi6n: get_ttyp ()->pcon_start =3D false; get_ttyp ()->pcon_activated =3D false; ReleaseMutex (input_mutex); - ReleaseMutex (pipe_sw_mutex); maybe_dumb: get_ttyp ()->pcon_cap_checked =3D true; return false; @@ -4314,7 +4331,6 @@ fhandler_pty_slave::cleanup_for_non_cygwin_app (handl= e_set_t *p, tty *ttyp, DWORD force_switch_to) { ttyp->wait_fwd (); - WaitForSingleObject (p->pipe_sw_mutex, INFINITE); if (nat_pipe_owner_self (ttyp->nat_pipe_owner_pid)) { DWORD switch_to =3D get_winpid_to_hand_over (ttyp, force_switch_to); @@ -4330,6 +4346,7 @@ fhandler_pty_slave::cleanup_for_non_cygwin_app (handl= e_set_t *p, tty *ttyp, ReleaseMutex (p->input_mutex); } } + WaitForSingleObject (p->pipe_sw_mutex, INFINITE); if (ttyp->pcon_activated) close_pseudoconsole (ttyp, force_switch_to); else @@ -4348,6 +4365,7 @@ fhandler_pty_slave::setpgid_aux (pid_t pid) if (!was_nat_fg && nat_fg && get_ttyp ()->switch_to_nat_pipe && get_ttyp ()->pty_input_state_eq (tty::to_cyg)) { + ReleaseMutex (pipe_sw_mutex); WaitForSingleObject (input_mutex, mutex_timeout); acquire_attach_mutex (mutex_timeout); transfer_input (tty::to_nat, get_handle (), get_ttyp (), @@ -4358,6 +4376,7 @@ fhandler_pty_slave::setpgid_aux (pid_t pid) else if (was_nat_fg && !nat_fg && get_ttyp ()->switch_to_nat_pipe && get_ttyp ()->pty_input_state_eq (tty::to_nat)) { + ReleaseMutex (pipe_sw_mutex); bool attach_restore =3D false; HANDLE from =3D get_handle_nat (); DWORD resume_pid =3D 0; @@ -4385,7 +4404,8 @@ fhandler_pty_slave::setpgid_aux (pid_t pid) release_attach_mutex (); ReleaseMutex (input_mutex); } - ReleaseMutex (pipe_sw_mutex); + else + ReleaseMutex (pipe_sw_mutex); } =20 bool