The queue is cleaned up by removing the entries having si_signo == 0 while processing the queued signals, however, sigpacket::process() may set si_signo in the queue to 0 of the entry already processed but not succeed by calling sig_clear(). This patch ensures the sig_clear() to remove the entry from the queue chain. For this purpose, the pointer prev has been added to the sigpacket. This is to handle the following case appropriately.
Consider the queued signal chain of: A->B->C->D without pointer prev. Assume that the pointer 'q' and 'qnext' point to C, and process() is processing C. If B is cleared in process(), A->next should be set to to C in sigpacket::clear(). Then, if process() for C succeeds, C should be removed from the queue, so A->next should be set to D. However, we cannot do that because we do not have the pointer to A in the while loop in wait_sig(). With the pointer prev, we can easily access A and C in sigpacket::clear() as well as A and D in the while loop in wait_sig() using the pointer prev and next without pursuing the chain. Addresses: https://cygwin.com/pipermail/cygwin/2024-November/256744.html Fixes: 9d2155089e87 ("(wait_sig): Define variable q to be the start of the signal queue. Just iterate through sigq queue, deleting processed or zeroed signals") Reported-by: Christian Franke <christian.fra...@t-online.de> Reviewed-by: Corinna Vinschen <cori...@vinschen.de> Signed-off-by: Takashi Yano <takashi.y...@nifty.ne.jp> --- winsup/cygwin/local_includes/sigproc.h | 3 +- winsup/cygwin/sigproc.cc | 48 +++++++++++++++++--------- 2 files changed, 34 insertions(+), 17 deletions(-) diff --git a/winsup/cygwin/local_includes/sigproc.h b/winsup/cygwin/local_includes/sigproc.h index 46e26db19..8b7062aae 100644 --- a/winsup/cygwin/local_includes/sigproc.h +++ b/winsup/cygwin/local_includes/sigproc.h @@ -50,8 +50,9 @@ struct sigpacket { HANDLE wakeup; HANDLE thread_handle; - struct sigpacket *next; }; + struct sigpacket *next; + struct sigpacket *prev; int process (); int setup_handler (void *, struct sigaction&, _cygtls *); }; diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc index 4d50a5865..8ffb90a2c 100644 --- a/winsup/cygwin/sigproc.cc +++ b/winsup/cygwin/sigproc.cc @@ -111,7 +111,7 @@ class pending_signals public: void add (sigpacket&); bool pending () {retry = !!start.next; return retry;} - void clear (int sig) {sigs[sig].si.si_signo = 0;} + void clear (int sig); void clear (_cygtls *tls); friend void sig_dispatch_pending (bool); friend void wait_sig (VOID *arg); @@ -432,21 +432,35 @@ sig_clear (int sig) sigq.clear (sig); } +/* Clear pending signals of specific si_signo. + Called from sigpacket::process(). */ +void +pending_signals::clear (int sig) +{ + sigpacket *q = sigs + sig; + if (!sig || !q->si.si_signo) + return; + q->si.si_signo = 0; + q->prev->next = q->next; + if (q->next) + q->next->prev = q->prev; +} + /* Clear pending signals of specific thread. Called under TLS lock from _cygtls::remove_pending_sigs. */ void pending_signals::clear (_cygtls *tls) { - sigpacket *q = &start, *qnext; + sigpacket *q = &start; - while ((qnext = q->next)) - if (qnext->sigtls == tls) + while ((q = q->next)) + if (q->sigtls == tls) { - qnext->si.si_signo = 0; - q->next = qnext->next; + q->si.si_signo = 0; + q->prev->next = q->next; + if (q->next) + q->next->prev = q->prev; } - else - q = qnext; } /* Clear pending signals of specific thread. Called from _cygtls::remove */ @@ -1299,7 +1313,10 @@ pending_signals::add (sigpacket& pack) return; *se = pack; se->next = start.next; - start.next = se; + se->prev = &start; + se->prev->next = se; + if (se->next) + se->next->prev = se; } /* Process signals by waiting for signal data to arrive in a pipe. @@ -1450,17 +1467,16 @@ wait_sig (VOID *) case __SIGFLUSHFAST: if (!sig_held) { - sigpacket *qnext; /* Check the queue for signals. There will always be at least one thing on the queue if this was a valid signal. */ - while ((qnext = q->next)) + while ((q = q->next)) { - if (qnext->si.si_signo && qnext->process () <= 0) - q = qnext; - else + if (q->si.si_signo && q->process () > 0) { - q->next = qnext->next; - qnext->si.si_signo = 0; + q->si.si_signo = 0; + q->prev->next = q->next; + if (q->next) + q->next->prev = q->prev; } } /* At least one signal still queued? The event is used in select -- 2.45.1
