[newlib-cygwin/cygwin-3_5-branch] Cygwin: console: Fix a race issue between close() and open().

[newlib-cygwin/cygwin-3_5-branch] Cygwin: console: Fix a race issue between close() and open().

[Takashi Yano]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=fc5e9525453fea7c27b0e13635ae54abaa0db69d

commit fc5e9525453fea7c27b0e13635ae54abaa0db69d
Author: Takashi Yano <takashi.yano@nifty.ne.jp>
Date:   Mon Mar 4 11:32:34 2024 +0900

    Cygwin: console: Fix a race issue between close() and open().
    
    The open() call for console sometimes fails if the console owner
    process is closing the console by close() at the same time. This
    is due to mismatch state of con.owner variable and attaching state
    to the console. With this patch, checking con.owner and attaching
    to con.owner sequence in open(), and resetting con.owner and freeing
    console sequence in close() are guarded by output_mutex to avoid
    such a race issue.
    Addresses: https://cygwin.com/pipermail/cygwin/2024-March/255575.html
    
    Fixes: 3721a756b0d8 ("Cygwin: console: Make the console accessible from other terminals.")
    Reported-by: Kate Deplaix <kit-ty-kate@outlook.com>
    Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>

Diff:
---
 winsup/cygwin/fhandler/console.cc | 58 +++++++++++++++++++++++----------------
 winsup/cygwin/release/3.5.2       |  4 +++
 2 files changed, 38 insertions(+), 24 deletions(-)

diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index 67ea95466..1c0d5c815 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -687,14 +687,6 @@ fhandler_console::set_unit ()
     {
       devset = (fh_devices) shared_console_info[unit]->tty_min_state.getntty ();
       _tc = &(shared_console_info[unit]->tty_min_state);
-      if (!created)
-	{
-	  while (con.owner > MAX_PID)
-	    Sleep (1);
-	  pinfo p (con.owner);
-	  if (!p)
-	    con.owner = myself->pid;
-	}
     }
 
   dev ().parse (devset);
@@ -1744,11 +1736,23 @@ fhandler_console::open (int flags, mode_t)
   set_handle (NULL);
   set_output_handle (NULL);
 
+  setup_io_mutex ();
+  acquire_output_mutex (mutex_timeout);
+
+  do
+    {
+      pinfo p (con.owner);
+      if (!p)
+	con.owner = myself->pid;
+    }
+  while (false);
+
   /* Open the input handle as handle_ */
   bool err = false;
   DWORD resume_pid = attach_console (con.owner, &err);
   if (err)
     {
+      release_output_mutex ();
       set_errno (EACCES);
       return 0;
     }
@@ -1759,6 +1763,7 @@ fhandler_console::open (int flags, mode_t)
 
   if (h == INVALID_HANDLE_VALUE)
     {
+      release_output_mutex ();
       __seterrno ();
       return 0;
     }
@@ -1768,6 +1773,7 @@ fhandler_console::open (int flags, mode_t)
   resume_pid = attach_console (con.owner, &err);
   if (err)
     {
+      release_output_mutex ();
       set_errno (EACCES);
       return 0;
     }
@@ -1778,14 +1784,16 @@ fhandler_console::open (int flags, mode_t)
 
   if (h == INVALID_HANDLE_VALUE)
     {
+      release_output_mutex ();
       __seterrno ();
       return 0;
     }
   set_output_handle (h);
   handle_set.output_handle = h;
+  release_output_mutex ();
+
   wpbuf.init ();
 
-  setup_io_mutex ();
   handle_set.input_mutex = input_mutex;
   handle_set.output_mutex = output_mutex;
 
@@ -1895,25 +1903,20 @@ fhandler_console::close ()
 	}
     }
 
-  release_output_mutex ();
-
-  if (shared_console_info[unit] && con.owner == myself->pid
-      && master_thread_started)
+  if (shared_console_info[unit] && con.owner == myself->pid)
     {
-      char name[MAX_PATH];
-      shared_name (name, CONS_THREAD_SYNC, get_minor ());
-      thread_sync_event = OpenEvent (MAXIMUM_ALLOWED, FALSE, name);
-      con.owner = MAX_PID + 1;
-      WaitForSingleObject (thread_sync_event, INFINITE);
-      CloseHandle (thread_sync_event);
+      if (master_thread_started)
+	{
+	  char name[MAX_PATH];
+	  shared_name (name, CONS_THREAD_SYNC, get_minor ());
+	  thread_sync_event = OpenEvent (MAXIMUM_ALLOWED, FALSE, name);
+	  con.owner = MAX_PID + 1;
+	  WaitForSingleObject (thread_sync_event, INFINITE);
+	  CloseHandle (thread_sync_event);
+	}
       con.owner = 0;
     }
 
-  CloseHandle (input_mutex);
-  input_mutex = NULL;
-  CloseHandle (output_mutex);
-  output_mutex = NULL;
-
   CloseHandle (get_handle ());
   CloseHandle (get_output_handle ());
 
@@ -1926,6 +1929,13 @@ fhandler_console::close ()
 	  || get_device () == (dev_t) myself->ctty))
     free_console ();
 
+  release_output_mutex ();
+
+  CloseHandle (input_mutex);
+  input_mutex = NULL;
+  CloseHandle (output_mutex);
+  output_mutex = NULL;
+
   if (shared_console_info[unit] && myself->ctty != tc ()->ntty)
     {
       UnmapViewOfFile ((void *) shared_console_info[unit]);
diff --git a/winsup/cygwin/release/3.5.2 b/winsup/cygwin/release/3.5.2
index 7d8df9489..fd3c768de 100644
--- a/winsup/cygwin/release/3.5.2
+++ b/winsup/cygwin/release/3.5.2
@@ -5,3 +5,7 @@ Fixes:
   is already unmapped due to race condition. To avoid this issue,
   shared console memory will be kept mapped if it belongs to CTTY.
   Addresses:  https://cygwin.com/pipermail/cygwin/2024-February/255561.html
+
+- Fix a race issue between console open() and close() which is caused
+  by state mismatch between con.owner and console attaching state.
+  Addresses: https://cygwin.com/pipermail/cygwin/2024-March/255575.html