[PATCH v3 5/6] Cygwin: signal: Use context locally copied in call_signal_handler()

Takashi Yano takashi.yano@nifty.ne.jp
Wed Mar 12 03:27:31 GMT 2025


If the signal handler is called from inside of another signal handler,
_cygtls::context may be destroyed by call_signal_handler() newly called.
To avoid this situation, this patch used context locally copied in
call_signal_handler().

Addresses: https://cygwin.com/pipermail/cygwin-patches/2025q1/013461.html
Fixes: 9043956ce859 ("Only construct ucontext for SA_SIGINFO signal handlers")
Reported-by: Takashi Yano <takashi.yano@nifty.ne.jp>
Reviewed-by:
Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
---
 winsup/cygwin/exceptions.cc | 41 ++++++++++++++++++++-----------------
 1 file changed, 22 insertions(+), 19 deletions(-)

diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc
index 18a566c45..1e19af68c 100644
--- a/winsup/cygwin/exceptions.cc
+++ b/winsup/cygwin/exceptions.cc
@@ -1660,6 +1660,8 @@ altstack_wrapper (int sig, siginfo_t *siginfo, ucontext_t *sigctx,
 int
 _cygtls::call_signal_handler ()
 {
+  ucontext_t context1 = context;
+
   int this_sa_flags = SA_RESTART;
   while (1)
     {
@@ -1697,10 +1699,10 @@ _cygtls::call_signal_handler ()
       /* Only make a context for SA_SIGINFO handlers */
       if (this_sa_flags & SA_SIGINFO)
 	{
-	  context.uc_link = 0;
-	  context.uc_flags = 0;
+	  context1.uc_link = 0;
+	  context1.uc_flags = 0;
 	  if (thissi.si_cyg)
-	    memcpy (&context.uc_mcontext,
+	    memcpy (&context1.uc_mcontext,
 		    ((cygwin_exception *) thissi.si_cyg)->context (),
 		    sizeof (CONTEXT));
 	  else
@@ -1710,13 +1712,13 @@ _cygtls::call_signal_handler ()
 		 from sigdelayed, fix the instruction pointer accordingly. */
 #pragma GCC diagnostic push
 #pragma GCC diagnostic ignored "-Wmaybe-uninitialized"
-	      RtlCaptureContext ((PCONTEXT) &context.uc_mcontext);
+	      RtlCaptureContext ((PCONTEXT) &context1.uc_mcontext);
 #pragma GCC diagnostic pop
-	      __unwind_single_frame ((PCONTEXT) &context.uc_mcontext);
+	      __unwind_single_frame ((PCONTEXT) &context1.uc_mcontext);
 	      if (stackptr > stack)
 		{
 #ifdef __x86_64__
-		  context.uc_mcontext.rip = retaddr ();
+		  context1.uc_mcontext.rip = retaddr ();
 #else
 #error unimplemented for this target
 #endif
@@ -1727,30 +1729,30 @@ _cygtls::call_signal_handler ()
 	      && !_my_tls.altstack.ss_flags
 	      && _my_tls.altstack.ss_sp)
 	    {
-	      context.uc_stack = _my_tls.altstack;
-	      context.uc_stack.ss_flags = SS_ONSTACK;
+	      context1.uc_stack = _my_tls.altstack;
+	      context1.uc_stack.ss_flags = SS_ONSTACK;
 	    }
 	  else
 	    {
-	      context.uc_stack.ss_sp = NtCurrentTeb ()->Tib.StackBase;
-	      context.uc_stack.ss_flags = 0;
+	      context1.uc_stack.ss_sp = NtCurrentTeb ()->Tib.StackBase;
+	      context1.uc_stack.ss_flags = 0;
 	      if (!NtCurrentTeb ()->DeallocationStack)
-		context.uc_stack.ss_size
+		context1.uc_stack.ss_size
 		  = (uintptr_t) NtCurrentTeb ()->Tib.StackLimit
 		    - (uintptr_t) NtCurrentTeb ()->Tib.StackBase;
 	      else
-		context.uc_stack.ss_size
+		context1.uc_stack.ss_size
 		  = (uintptr_t) NtCurrentTeb ()->DeallocationStack
 		    - (uintptr_t) NtCurrentTeb ()->Tib.StackBase;
 	    }
-	  context.uc_sigmask = context.uc_mcontext.oldmask = this_oldmask;
+	  context1.uc_sigmask = context1.uc_mcontext.oldmask = this_oldmask;
 
-	  context.uc_mcontext.cr2 = (thissi.si_signo == SIGSEGV
-				     || thissi.si_signo == SIGBUS)
-				    ? (uintptr_t) thissi.si_addr : 0;
+	  context1.uc_mcontext.cr2 = (thissi.si_signo == SIGSEGV
+				      || thissi.si_signo == SIGBUS)
+				     ? (uintptr_t) thissi.si_addr : 0;
 
-	  thiscontext = &context;
-	  context_copy = context;
+	  thiscontext = &context1;
+	  context_copy = context1;
 	}
 
       int this_errno = saved_errno;
@@ -1836,10 +1838,11 @@ _cygtls::call_signal_handler ()
       incyg = true;
 
       set_signal_mask (_my_tls.sigmask, (this_sa_flags & SA_SIGINFO)
-					? context.uc_sigmask : this_oldmask);
+					? context1.uc_sigmask : this_oldmask);
       if (this_errno >= 0)
 	set_errno (this_errno);
     }
+  context = context1;
 
   /* FIXME: Since 2011 this return statement always returned 1 (meaning
      SA_RESTART is effective) if the thread we're running in is not the
-- 
2.45.1



More information about the Cygwin-patches mailing list