Cygwin: signal: Fix a race issue on modifying _pinfo::process_state

The PID_STOPPED flag in _ponfo::process_state is sometimes accidentally
cleared due to a race condition when modifying it with the "|=" or "&="
operators. This patch uses InterlockedOr/And() instead to avoid the
race condition.

Addresses: https://cygwin.com/pipermail/cygwin/2025-February/257473.html
Fixes: 1fd5e000ace55 ("import winsup-2000-02-17 snapshot")
Reported-by: Christian Franke <Christian.Franke@t-online.de>
Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>

7 files changed, 23 insertions, 19 deletions

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index c6e82b6..45c71cb 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -882,7 +882,7 @@ sig_handle_tty_stop (int sig, siginfo_t *, void *)
   /* Silently ignore attempts to suspend if there is no accommodating
      cygwin parent to deal with this behavior. */
   if (!myself->cygstarted)
-    myself->process_state &= ~PID_STOPPED;
+    InterlockedAnd ((LONG *) &myself->process_state, ~PID_STOPPED);
   else
     {
       _my_tls.incyg = 1;
@@ -948,7 +948,7 @@ _cygtls::interrupt_setup (siginfo_t& si, void *handler, struct sigaction& siga)
   if (handler == sig_handle_tty_stop)
     {
       myself->stopsig = 0;
-      myself->process_state |= PID_STOPPED;
+      InterlockedOr ((LONG *) &myself->process_state, PID_STOPPED);
     }
 
   infodata = si;
@@ -1435,7 +1435,7 @@ _cygtls::handle_SIGCONT ()
     yield ();
 
   myself->stopsig = 0;
-  myself->process_state &= ~PID_STOPPED;
+  InterlockedAnd ((LONG *) &myself->process_state, ~PID_STOPPED);
 
   /* Clear pending stop signals */
   sig_clear (SIGSTOP, false);
diff --git a/winsup/cygwin/fork.cc b/winsup/cygwin/fork.cc
index 41a5337..783971b 100644
--- a/winsup/cygwin/fork.cc
+++ b/winsup/cygwin/fork.cc
@@ -678,8 +678,9 @@ dofork (void **proc, bool *with_forkables)
 
   if (ischild)
     {
-      myself->process_state |= PID_ACTIVE;
-      myself->process_state &= ~(PID_INITIALIZING | PID_EXITED | PID_REAPED);
+      InterlockedOr ((LONG *) &myself->process_state, PID_ACTIVE);
+      InterlockedAnd ((LONG *) &myself->process_state,
+		      ~(PID_INITIALIZING | PID_EXITED | PID_REAPED));
     }
   else if (res < 0)
     {
diff --git a/winsup/cygwin/local_includes/pinfo.h b/winsup/cygwin/local_includes/pinfo.h
index 4de0f80..d1c9b00 100644
--- a/winsup/cygwin/local_includes/pinfo.h
+++ b/winsup/cygwin/local_includes/pinfo.h
@@ -54,7 +54,7 @@ public:
 
   /* Various flags indicating the state of the process.  See PID_
      constants in <sys/cygwin.h>. */
-  DWORD process_state;
+  volatile DWORD process_state;
 
   pid_t ppid;		/* Parent process id.  */
 
@@ -271,9 +271,9 @@ public:
   push_process_state (int add_flag)
   {
     flag = add_flag;
-    myself->process_state |= flag;
+    InterlockedOr ((LONG *) &myself->process_state, flag);
   }
-  void pop () { myself->process_state &= ~(flag); }
+  void pop () { InterlockedAnd ((LONG *) &myself->process_state, ~(flag)); }
   ~push_process_state () { pop (); }
 };
 
diff --git a/winsup/cygwin/pinfo.cc b/winsup/cygwin/pinfo.cc
index 1f26a3c..c69213f 100644
--- a/winsup/cygwin/pinfo.cc
+++ b/winsup/cygwin/pinfo.cc
@@ -69,7 +69,7 @@ pinfo::thisproc (HANDLE h)
      child_info_spawn::handle_spawn. */
 
   init (cygheap->pid, flags, h);
-  procinfo->process_state |= PID_IN_USE;
+  InterlockedOr ((LONG *) &procinfo->process_state, PID_IN_USE);
   procinfo->dwProcessId = myself_initial.dwProcessId;
   procinfo->sendsig = myself_initial.sendsig;
   wcscpy (procinfo->progname, myself_initial.progname);
@@ -89,7 +89,7 @@ pinfo_init (char **envp, int envc)
     {
       environ_init (envp, envc);
       /* spawn has already set up a pid structure for us so we'll use that */
-      myself->process_state |= PID_CYGPARENT;
+      InterlockedOr ((LONG *) &myself->process_state, PID_CYGPARENT);
     }
   else
     {
@@ -108,10 +108,11 @@ pinfo_init (char **envp, int envc)
       debug_printf ("Set nice to %d", myself->nice);
     }
 
-  myself->process_state |= PID_ACTIVE;
-  myself->process_state &= ~(PID_INITIALIZING | PID_EXITED | PID_REAPED);
+  InterlockedOr ((LONG *) &myself->process_state, PID_ACTIVE);
+  InterlockedAnd ((LONG *) &myself->process_state,
+		  ~(PID_INITIALIZING | PID_EXITED | PID_REAPED));
   if (being_debugged ())
-    myself->process_state |= PID_DEBUGGED;
+    InterlockedOr ((LONG *) &myself->process_state, PID_DEBUGGED);
   myself.preserve ();
   debug_printf ("pid %d, pgid %d, process_state %y",
 		myself->pid, myself->pgid, myself->process_state);
diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc
index 0bd6496..f8ba67e 100644
--- a/winsup/cygwin/signal.cc
+++ b/winsup/cygwin/signal.cc
@@ -456,9 +456,11 @@ sigaction_worker (int sig, const struct sigaction *newact,
 		sig_clear (sig, true);
 	      if (sig == SIGCHLD)
 		{
-		  myself->process_state &= ~PID_NOCLDSTOP;
+		  InterlockedAnd ((LONG *)&myself->process_state,
+				  ~PID_NOCLDSTOP);
 		  if (gs.sa_flags & SA_NOCLDSTOP)
-		    myself->process_state |= PID_NOCLDSTOP;
+		    InterlockedOr ((LONG *) &myself->process_state,
+				   PID_NOCLDSTOP);
 		}
 	    }
 
diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc
index 1ffe00a..8739f18 100644
--- a/winsup/cygwin/sigproc.cc
+++ b/winsup/cygwin/sigproc.cc
@@ -252,7 +252,7 @@ proc_subproc (DWORD what, uintptr_t val)
 	  vchild->sid = myself->sid;
 	  vchild->ctty = myself->ctty;
 	  vchild->cygstarted = true;
-	  vchild->process_state |= PID_INITIALIZING;
+	  InterlockedOr ((LONG *)&vchild->process_state, PID_INITIALIZING);
 	  vchild->ppid = myself->pid;	/* always set last */
 	}
       break;
diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc
index 8016f08..06b8423 100644
--- a/winsup/cygwin/spawn.cc
+++ b/winsup/cygwin/spawn.cc
@@ -543,7 +543,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       refresh_cygheap ();
 
       if (c_flags & CREATE_NEW_PROCESS_GROUP)
-	myself->process_state |= PID_NEW_PG;
+	InterlockedOr ((LONG *) &myself->process_state, PID_NEW_PG);
 
       if (mode == _P_DETACH)
 	/* all set */;
@@ -603,7 +603,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
       ::cygheap->user.deimpersonate ();
 
       if (!real_path.iscygexec () && mode == _P_OVERLAY)
-	myself->process_state |= PID_NOTCYGWIN;
+	InterlockedOr ((LONG *) &myself->process_state, PID_NOTCYGWIN);
 
       cygpid = (mode != _P_OVERLAY) ? create_cygwin_pid () : myself->pid;
 
@@ -705,7 +705,7 @@ child_info_spawn::worker (const char *prog_arg, const char *const *argv,
 	      myself->sendsig = myself->exec_sendsig;
 	      myself->exec_sendsig = NULL;
 	    }
-	  myself->process_state &= ~PID_NOTCYGWIN;
+	  InterlockedAnd ((LONG *) &myself->process_state, ~PID_NOTCYGWIN);
 	  /* Reset handle inheritance to default when the execution of a'
 	     non-Cygwin process fails.  Only need to do this for _P_OVERLAY
 	     since the handle will be closed otherwise.  Don't need to do