public inbox for cygwin-patches@cygwin.com
 help / color / mirror / Atom feed
From: Takashi Yano <takashi.yano@nifty.ne.jp>
To: cygwin-patches@cygwin.com
Subject: [PATCH] Cygwin: pty: Additional race issue fix regarding pseudo console.
Date: Wed, 21 Apr 2021 12:06:00 +0900	[thread overview]
Message-ID: <20210421030600.3793-1-takashi.yano@nifty.ne.jp> (raw)

- In commit bb93c6d7, the race issue was not completely fixed. In
  the pseudo console inheritance, if the destination process to
  which the ownership of pseudo console switches, is found but exits
  before switching, the inheritance fails. Currently, this extremely
  rarely happens. This patch fixes the issue.
---
 winsup/cygwin/fhandler_tty.cc | 47 +++++++++++------------------------
 1 file changed, 14 insertions(+), 33 deletions(-)

diff --git a/winsup/cygwin/fhandler_tty.cc b/winsup/cygwin/fhandler_tty.cc
index d44728795..e4480771b 100644
--- a/winsup/cygwin/fhandler_tty.cc
+++ b/winsup/cygwin/fhandler_tty.cc
@@ -64,6 +64,8 @@ struct pipe_reply {
 
 extern HANDLE attach_mutex; /* Defined in fhandler_console.cc */
 
+inline static bool pcon_pid_alive (DWORD pid);
+
 DWORD
 fhandler_pty_common::get_console_process_id (DWORD pid, bool match,
 					     bool cygwin, bool stub_only)
@@ -90,20 +92,18 @@ fhandler_pty_common::get_console_process_id (DWORD pid, bool match,
 	else
 	  {
 	    pinfo p (cygwin_pid (list[i]));
-	    if (!!p && p->dwProcessId && p->exec_dwProcessId
-		&& p->dwProcessId != p->exec_dwProcessId)
+	    if (!!p && p->exec_dwProcessId)
 	      {
-		res_pri = list[i];
+		res_pri = stub_only ? p->exec_dwProcessId : list[i];
 		break;
 	      }
-	    if (!!p && !res)
+	    if (!p && !res && pcon_pid_alive (list[i]) && stub_only)
+	      res = list[i];
+	    if (!!p && !res && !stub_only)
 	      res = list[i];
 	  }
       }
-  if (stub_only)
-    return res_pri;
-  else
-    return res_pri ?: res;
+  return res_pri ?: res;
 }
 
 static bool isHybrid;
@@ -3384,35 +3384,17 @@ fallback:
 void
 fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
 {
-  DWORD switch_to_stub = 0, switch_to = 0;
-  DWORD new_pcon_pid = 0;
+  DWORD switch_to = 0;
   if (force_switch_to)
     {
-      switch_to_stub = force_switch_to;
-      new_pcon_pid = force_switch_to;
+      switch_to = force_switch_to;
       ttyp->setpgid (force_switch_to + MAX_PID);
     }
   else if (pcon_pid_self (ttyp->pcon_pid))
     {
       /* Search another process which attaches to the pseudo console */
       DWORD current_pid = myself->exec_dwProcessId ?: myself->dwProcessId;
-      switch_to = get_console_process_id (current_pid, false, true);
-      if (switch_to)
-	{
-	  pinfo p (cygwin_pid (switch_to));
-	  if (p)
-	    {
-	      if (p->exec_dwProcessId)
-		switch_to_stub = p->exec_dwProcessId;
-	      new_pcon_pid = p->exec_dwProcessId;
-	    }
-	}
-      else
-	{
-	  switch_to = get_console_process_id (current_pid, false, false);
-	  if (switch_to)
-	    new_pcon_pid = switch_to;
-	}
+      switch_to = get_console_process_id (current_pid, false, true, true);
     }
   if (ttyp->pcon_activated)
     {
@@ -3420,7 +3402,6 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
       ttyp->previous_output_code_page = GetConsoleOutputCP ();
       if (pcon_pid_self (ttyp->pcon_pid))
 	{
-	  switch_to = switch_to_stub ?: switch_to;
 	  if (switch_to)
 	    {
 	      /* Change pseudo console owner to another process */
@@ -3454,7 +3435,7 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
 	      CloseHandle (ttyp->h_pcon_conhost_process);
 	      CloseHandle (ttyp->h_pcon_in);
 	      CloseHandle (ttyp->h_pcon_out);
-	      ttyp->pcon_pid = new_pcon_pid;
+	      ttyp->pcon_pid = switch_to;
 	      ttyp->h_pcon_write_pipe = new_write_pipe;
 	      ttyp->h_pcon_condrv_reference = new_condrv_reference;
 	      ttyp->h_pcon_conhost_process = new_conhost_process;
@@ -3513,8 +3494,8 @@ fhandler_pty_slave::close_pseudoconsole (tty *ttyp, DWORD force_switch_to)
     }
   else if (pcon_pid_self (ttyp->pcon_pid))
     {
-      if (switch_to_stub)
-	ttyp->pcon_pid = new_pcon_pid;
+      if (switch_to)
+	ttyp->pcon_pid = switch_to;
       else
 	{
 	  ttyp->pcon_pid = 0;
-- 
2.31.1


             reply	other threads:[~2021-04-21  3:06 UTC|newest]

Thread overview: 2+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-04-21  3:06 Takashi Yano [this message]
2021-04-21 15:44 ` Corinna Vinschen

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210421030600.3793-1-takashi.yano@nifty.ne.jp \
    --to=takashi.yano@nifty.ne.jp \
    --cc=cygwin-patches@cygwin.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).