[newlib-cygwin/cygwin-3_5-branch] Cygwin: signal: Remove queue entry from the queue chain when cleared

[Corinna Vinschen]

https://sourceware.org/git/gitweb.cgi?p=newlib-cygwin.git;h=5bd4f9936f20ef5f06dc11986271744bf6d87359

commit 5bd4f9936f20ef5f06dc11986271744bf6d87359
Author:     Takashi Yano <takashi.yano@nifty.ne.jp>
AuthorDate: Fri Nov 29 16:46:48 2024 +0900
Commit:     Corinna Vinschen <corinna@vinschen.de>
CommitDate: Fri Dec 6 11:43:22 2024 +0100

    Cygwin: signal: Remove queue entry from the queue chain when cleared
    
    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.Franke@t-online.de>
    Reviewed-by: Corinna Vinschen <corinna@vinschen.de>
    Signed-off-by: Takashi Yano <takashi.yano@nifty.ne.jp>
    (cherry picked from commit d565aca46f06117ef16ec37c51767a5e140ee9e2)

Diff:
---
 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 7aca80595dc9..41745f214c24 100644
--- a/winsup/cygwin/local_includes/sigproc.h
+++ b/winsup/cygwin/local_includes/sigproc.h
@@ -51,8 +51,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 b9669a861610..287e78ff521d 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 */
@@ -1300,7 +1314,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.
@@ -1455,17 +1472,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