From 496fa7b2ce0052550eab8900723ebb59c33d25e7 Mon Sep 17 00:00:00 2001 From: Takashi Yano Date: Wed, 4 Dec 2024 11:53:41 +0900 Subject: Cygwin: signal: Introduce a lock for the signal queue Currently, the signal queue is touched by the thread sig as well as other threads that call sigaction_worker(). This potentially has a possibility to destroy the signal queue chain. A possible worst result may be a self-loop chain which causes infinite loop. With this patch, lock()/unlock() are introduce to avoid such a situation. Fixes: 474048c26edf ("* sigproc.cc (pending_signals::add): Just index directly into signal array rather than treating the array as a heap.") Suggested-by: Corinna Vinschen Reviewed-by: Corinna Vinschen Signed-off-by: Takashi Yano --- winsup/cygwin/exceptions.cc | 12 ++++++------ winsup/cygwin/local_includes/sigproc.h | 2 +- winsup/cygwin/signal.cc | 4 ++-- winsup/cygwin/sigproc.cc | 28 +++++++++++++++++++++++----- 4 files changed, 32 insertions(+), 14 deletions(-) (limited to 'winsup') diff --git a/winsup/cygwin/exceptions.cc b/winsup/cygwin/exceptions.cc index 0f8c219..35a4a0b 100644 --- a/winsup/cygwin/exceptions.cc +++ b/winsup/cygwin/exceptions.cc @@ -1450,10 +1450,10 @@ _cygtls::handle_SIGCONT (threadlist_t * &tl_entry) sigsent = true; } /* Clear pending stop signals */ - sig_clear (SIGSTOP); - sig_clear (SIGTSTP); - sig_clear (SIGTTIN); - sig_clear (SIGTTOU); + sig_clear (SIGSTOP, false); + sig_clear (SIGTSTP, false); + sig_clear (SIGTTIN, false); + sig_clear (SIGTTOU, false); } int @@ -1554,14 +1554,14 @@ sigpacket::process () goto exit_sig; if (si.si_signo == SIGSTOP) { - sig_clear (SIGCONT); + sig_clear (SIGCONT, false); goto stop; } /* Clear pending SIGCONT on stop signals */ if (si.si_signo == SIGTSTP || si.si_signo == SIGTTIN || si.si_signo == SIGTTOU) - sig_clear (SIGCONT); + sig_clear (SIGCONT, false); if (handler == (void *) SIG_DFL) { diff --git a/winsup/cygwin/local_includes/sigproc.h b/winsup/cygwin/local_includes/sigproc.h index 8b7062a..ce72633 100644 --- a/winsup/cygwin/local_includes/sigproc.h +++ b/winsup/cygwin/local_includes/sigproc.h @@ -62,7 +62,7 @@ void set_signal_mask (sigset_t&, sigset_t); int handle_sigprocmask (int sig, const sigset_t *set, sigset_t *oldset, sigset_t& opmask); -void sig_clear (int); +void sig_clear (int, bool); void sig_set_pending (int); int handle_sigsuspend (sigset_t); diff --git a/winsup/cygwin/signal.cc b/winsup/cygwin/signal.cc index a7af604..0bd6496 100644 --- a/winsup/cygwin/signal.cc +++ b/winsup/cygwin/signal.cc @@ -451,9 +451,9 @@ sigaction_worker (int sig, const struct sigaction *newact, if (!(gs.sa_flags & SA_NODEFER)) gs.sa_mask |= SIGTOMASK(sig); if (gs.sa_handler == SIG_IGN) - sig_clear (sig); + sig_clear (sig, true); if (gs.sa_handler == SIG_DFL && sig == SIGCHLD) - sig_clear (sig); + sig_clear (sig, true); if (sig == SIGCHLD) { myself->process_state &= ~PID_NOCLDSTOP; diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc index 7e02e61..c4c1595 100644 --- a/winsup/cygwin/sigproc.cc +++ b/winsup/cygwin/sigproc.cc @@ -106,12 +106,16 @@ class pending_signals { sigpacket sigs[_NSIG + 1]; sigpacket start; + SRWLOCK queue_lock; bool retry; + void lock () { AcquireSRWLockExclusive (&queue_lock); } + void unlock () { ReleaseSRWLockExclusive (&queue_lock); } public: + pending_signals (): queue_lock (SRWLOCK_INIT) {} void add (sigpacket&); bool pending () {retry = !!start.next; return retry;} - void clear (int sig); + void clear (int sig, bool need_lock); void clear (_cygtls *tls); friend void sig_dispatch_pending (bool); friend void wait_sig (VOID *arg); @@ -427,23 +431,27 @@ proc_terminate () /* Clear pending signal */ void -sig_clear (int sig) +sig_clear (int sig, bool need_lock) { - sigq.clear (sig); + sigq.clear (sig, need_lock); } /* Clear pending signals of specific si_signo. Called from sigpacket::process(). */ void -pending_signals::clear (int sig) +pending_signals::clear (int sig, bool need_lock) { sigpacket *q = sigs + sig; if (!sig || !q->si.si_signo) return; + if (need_lock) + lock (); q->si.si_signo = 0; q->prev->next = q->next; if (q->next) q->next->prev = q->prev; + if (need_lock) + unlock (); } /* Clear pending signals of specific thread. Called under TLS lock from @@ -453,6 +461,7 @@ pending_signals::clear (_cygtls *tls) { sigpacket *q = &start; + lock (); while ((q = q->next)) if (q->sigtls == tls) { @@ -461,6 +470,7 @@ pending_signals::clear (_cygtls *tls) if (q->next) q->next->prev = q->prev; } + unlock (); } /* Clear pending signals of specific thread. Called from _cygtls::remove */ @@ -1313,11 +1323,13 @@ pending_signals::add (sigpacket& pack) if (se->si.si_signo) return; *se = pack; + lock (); se->next = start.next; se->prev = &start; se->prev->next = se; if (se->next) se->next->prev = se; + unlock (); } /* Process signals by waiting for signal data to arrive in a pipe. @@ -1398,6 +1410,7 @@ wait_sig (VOID *) bool issig_wait; *pack.mask = 0; + sigq.lock (); while ((q = q->next)) { _cygtls *sigtls = q->sigtls ?: _main_tls; @@ -1411,6 +1424,7 @@ wait_sig (VOID *) } } } + sigq.unlock (); } break; case __SIGPENDING: @@ -1419,6 +1433,7 @@ wait_sig (VOID *) *pack.mask = 0; tl_entry = cygheap->find_tls (pack.sigtls); + sigq.lock (); while ((q = q->next)) { /* Skip thread-specific signals for other threads. */ @@ -1427,6 +1442,7 @@ wait_sig (VOID *) if (pack.sigtls->sigmask & (bit = SIGTOMASK (q->si.si_signo))) *pack.mask |= bit; } + sigq.unlock (); cygheap->unlock_tls (tl_entry); } break; @@ -1461,7 +1477,7 @@ wait_sig (VOID *) break; default: /* Normal (positive) signal */ if (pack.si.si_signo < 0) - sig_clear (-pack.si.si_signo); + sig_clear (-pack.si.si_signo, true); else sigq.add (pack); fallthrough; @@ -1474,6 +1490,7 @@ wait_sig (VOID *) { /* Check the queue for signals. There will always be at least one thing on the queue if this was a valid signal. */ + sigq.lock (); while ((q = q->next)) { if (q->si.si_signo && q->process () > 0) @@ -1484,6 +1501,7 @@ wait_sig (VOID *) q->next->prev = q->prev; } } + sigq.unlock (); /* At least one signal still queued? The event is used in select only, and only to decide if WFMO should wake up in case a signalfd is waiting via select/poll for being ready to read a -- cgit v1.1