[PATCH] winsup: console: validate duplicated handle set for master thread

From 1d5fbfa8e1bf2593231417e8200f354240ad4ecc Mon Sep 17 00:00:00 2001
From: Chris Chen <[email protected]>
Date: Tue, 11 Aug 2026 04:48:27 +0800
Subject: [PATCH] winsup: console: validate duplicated handle set for master
 thread

The console master thread currently assumes that all handles in the
duplicated handle set were duplicated successfully.

If one of the DuplicateHandle calls fails, however, the master thread
may be started with a partially initialized handle set.  Failure to
duplicate thread_sync_event can also leave already duplicated console
handles unreleased.

Change get_duplicated_handle_set to return bool and treat the handle set
as an all-or-nothing resource.

Initialize the handle set before duplication, clean up handles already
acquired when a subsequent duplication fails, and do not start the
master thread unless the complete handle set is available.

Also release the duplicated console handles if duplication of
thread_sync_event fails, and make close_handle_set tolerate partially
initialized handle sets.

This change does not alter console synchronization semantics.  It only
hardens handle acquisition and cleanup on failure paths.
---
 winsup/cygwin/fhandler/console.cc       | 101 ++++++++++++++++++------
 winsup/cygwin/local_includes/fhandler.h |   2 +-
 2 files changed, 80 insertions(+), 23 deletions(-)

diff --git a/winsup/cygwin/fhandler/console.cc b/winsup/cygwin/fhandler/console.cc
index ba35ca44c..b4d18d4f7 100644
--- a/winsup/cygwin/fhandler/console.cc
+++ b/winsup/cygwin/fhandler/console.cc
@@ -282,9 +282,17 @@ cons_master_thread (VOID *arg)
 {
   fhandler_console *fh = (fhandler_console *) arg;
   tty *ttyp = (tty *) fh->tc ();
-  fhandler_console::handle_set_t handle_set;
-  fh->get_duplicated_handle_set (&handle_set);
-  HANDLE thread_sync_event;
+  fhandler_console::handle_set_t handle_set = {};
+
+  /* Require a complete duplicated console handle set. */
+  if (!fh->get_duplicated_handle_set (&handle_set))
+    {
+      debug_printf ("cons_master_thread not started because console "
+		    "handle duplication failed");
+      return 0;
+    }
+
+  HANDLE thread_sync_event = NULL;
   if (DuplicateHandle (GetCurrentProcess (), fh->thread_sync_event,
 		       GetCurrentProcess (), &thread_sync_event,
 		       0, FALSE, DUPLICATE_SAME_ACCESS))
@@ -300,8 +308,13 @@ cons_master_thread (VOID *arg)
       master_thread_started = false;
     }
   else
-    debug_printf ("cons_master_thread not started because thread_sync_event "
-		  "could not be duplicated %08x", GetLastError ());
+    {
+      DWORD err = GetLastError ();
+      /* Release duplicated console handles if sync-event duplication fails. */
+      fhandler_console::close_handle_set (&handle_set);
+      debug_printf ("cons_master_thread not started because thread_sync_event "
+		    "could not be duplicated %08x", err);
+    }
   return 0;
 }
 
@@ -4803,22 +4816,58 @@ fhandler_console::__release_output_mutex (const char *fn, int ln)
 #endif
 }
 
-void
+bool
 fhandler_console::get_duplicated_handle_set (handle_set_t *p)
 {
-  DuplicateHandle (GetCurrentProcess (), get_handle (),
-		   GetCurrentProcess (), &p->input_handle,
-		   0, FALSE, DUPLICATE_SAME_ACCESS);
-  DuplicateHandle (GetCurrentProcess (), get_output_handle (),
-		   GetCurrentProcess (), &p->output_handle,
-		   0, FALSE, DUPLICATE_SAME_ACCESS);
-  DuplicateHandle (GetCurrentProcess (), input_mutex,
-		   GetCurrentProcess (), &p->input_mutex,
-		   0, FALSE, DUPLICATE_SAME_ACCESS);
-  DuplicateHandle (GetCurrentProcess (), output_mutex,
-		   GetCurrentProcess (), &p->output_mutex,
-		   0, FALSE, DUPLICATE_SAME_ACCESS);
+  /* Validate duplicated console handles before starting the master thread.
+     A master thread must never start with a partial handle set. */
+  p->input_handle = NULL;
+  p->output_handle = NULL;
+  p->input_mutex = NULL;
+  p->output_mutex = NULL;
   p->unit = unit;
+
+  if (!DuplicateHandle (GetCurrentProcess (), get_handle (),
+			GetCurrentProcess (), &p->input_handle,
+			0, FALSE, DUPLICATE_SAME_ACCESS))
+    {
+      debug_printf ("DuplicateHandle(input_handle %p) failed %08x",
+		    get_handle (), GetLastError ());
+      goto fail;
+    }
+
+  if (!DuplicateHandle (GetCurrentProcess (), get_output_handle (),
+			GetCurrentProcess (), &p->output_handle,
+			0, FALSE, DUPLICATE_SAME_ACCESS))
+    {
+      debug_printf ("DuplicateHandle(output_handle %p) failed %08x",
+		    get_output_handle (), GetLastError ());
+      goto fail;
+    }
+
+  if (!DuplicateHandle (GetCurrentProcess (), input_mutex,
+			GetCurrentProcess (), &p->input_mutex,
+			0, FALSE, DUPLICATE_SAME_ACCESS))
+    {
+      debug_printf ("DuplicateHandle(input_mutex %p) failed %08x",
+		    input_mutex, GetLastError ());
+      goto fail;
+    }
+
+  if (!DuplicateHandle (GetCurrentProcess (), output_mutex,
+			GetCurrentProcess (), &p->output_mutex,
+			0, FALSE, DUPLICATE_SAME_ACCESS))
+    {
+      debug_printf ("DuplicateHandle(output_mutex %p) failed %08x",
+		    output_mutex, GetLastError ());
+      goto fail;
+    }
+
+  return true;
+
+fail:
+  close_handle_set (p);
+  return false;
 }
 
 /* The function close_handle_set() should be static so that they can
@@ -4826,13 +4875,21 @@ fhandler_console::get_duplicated_handle_set (handle_set_t *p)
 void
 fhandler_console::close_handle_set (handle_set_t *p)
 {
-  CloseHandle (p->input_handle);
+  /* Close only console handles that were successfully duplicated. */
+  if (p->input_handle)
+    CloseHandle (p->input_handle);
   p->input_handle = NULL;
-  CloseHandle (p->output_handle);
+
+  if (p->output_handle)
+    CloseHandle (p->output_handle);
   p->output_handle = NULL;
-  CloseHandle (p->input_mutex);
+
+  if (p->input_mutex)
+    CloseHandle (p->input_mutex);
   p->input_mutex = NULL;
-  CloseHandle (p->output_mutex);
+
+  if (p->output_mutex)
+    CloseHandle (p->output_mutex);
   p->output_mutex = NULL;
 }
 
diff --git a/winsup/cygwin/local_includes/fhandler.h b/winsup/cygwin/local_includes/fhandler.h
index d11b3ec4f..a9683637d 100644
--- a/winsup/cygwin/local_includes/fhandler.h
+++ b/winsup/cygwin/local_includes/fhandler.h
@@ -2350,7 +2350,7 @@ private:
   size_t &raixput ();
   size_t &rabuflen ();
 
-  void get_duplicated_handle_set (handle_set_t *p);
+  bool get_duplicated_handle_set (handle_set_t *p);
   static void close_handle_set (handle_set_t *p);
 
   static void cons_master_thread (handle_set_t *p, tty *ttyp);
-- 
2.55.0.windows.3