signal: fix deadlock when sigdeliver call enter_critical_section

cpu0                                 cpu1:

user_main
signest_test
sched_unlock
nxsched_merge_pending
nxsched_add_readytorun
up_cpu_pause
			             arm_sigdeliver
				     enter_critical_section

Reason:
In the SMP, cpu0 is already in the critical section and waiting for cpu1 to enter the suspended state.
However, when cpu1 executes arm_sigdeliver, it is in the irq-disabled state but not in the critical section.
At this point, cpu1 is unable to respond to interrupts and
is continuously attempting to enter the critical section, resulting in a deadlock.

Resolve:
adjust the logic, do not entering the critical section when interrupt-disabled.

test:
We can use qemu for testing.

compiling
make distclean -j20; ./tools/configure.sh -l qemu-armv8a:nsh_smp ;make -j20
running
qemu-system-aarch64 -cpu cortex-a53 -smp 4 -nographic -machine virt,virtualization=on,gic-version=3 -net none -chardev stdio,id=con,mux=on -serial chardev:con -mon chardev=con,mode=readline -kernel ./nuttx

Signed-off-by: hujun5 <hujun5@xiaomi.com>

arch/arm/src/armv6-m/arm_sigdeliver.c

@@ -63,8 +63,6 @@ void arm_sigdeliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -81,17 +79,16 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section((uint16_t)regs[REG_PRIMASK]);
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -122,7 +119,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -135,6 +132,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section((uint16_t)regs[REG_PRIMASK]);
+#endif
       goto retry;
     }
 

arch/arm/src/armv7-a/arm_sigdeliver.c

@@ -63,8 +63,6 @@ void arm_sigdeliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -81,17 +79,16 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section(regs[REG_CPSR]);
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -122,7 +119,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -135,6 +132,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section(regs[REG_CPSR]);
+#endif
       goto retry;
     }
 

arch/arm/src/armv7-m/arm_sigdeliver.c

@@ -63,8 +63,6 @@ void arm_sigdeliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -81,13 +79,13 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
 #ifdef CONFIG_ARMV7M_USEBASEPRI
       leave_critical_section((uint8_t)regs[REG_BASEPRI]);
@@ -95,7 +93,6 @@ retry:
       leave_critical_section((uint16_t)regs[REG_PRIMASK]);
 #endif
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -126,7 +123,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -139,6 +136,13 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+#  ifdef CONFIG_ARMV7M_USEBASEPRI
+      leave_critical_section((uint8_t)regs[REG_BASEPRI]);
+#  else
+      leave_critical_section((uint16_t)regs[REG_PRIMASK]);
+#  endif
+#endif
       goto retry;
     }
 

arch/arm/src/armv7-r/arm_sigdeliver.c

@@ -63,8 +63,6 @@ void arm_sigdeliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -81,17 +79,16 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section(regs[REG_CPSR]);
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -119,7 +116,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -132,6 +129,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section(regs[REG_CPSR]);
+#endif
       goto retry;
     }
 

arch/arm/src/armv8-m/arm_sigdeliver.c

@@ -63,8 +63,6 @@ void arm_sigdeliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -81,13 +79,13 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
 #ifdef CONFIG_ARMV8M_USEBASEPRI
       leave_critical_section((uint8_t)regs[REG_BASEPRI]);
@@ -95,7 +93,6 @@ retry:
       leave_critical_section((uint16_t)regs[REG_PRIMASK]);
 #endif
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -126,7 +123,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -139,6 +136,13 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+#  ifdef CONFIG_ARMV8M_USEBASEPRI
+      leave_critical_section((uint8_t)regs[REG_BASEPRI]);
+#  else
+      leave_critical_section((uint16_t)regs[REG_PRIMASK]);
+#  endif
+#endif
       goto retry;
     }
 

arch/arm/src/armv8-r/arm_sigdeliver.c

@@ -79,17 +79,16 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section(regs[REG_CPSR]);
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -117,7 +116,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -130,6 +129,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section(regs[REG_CPSR]);
+#endif
       goto retry;
     }
 
@@ -149,6 +151,8 @@ retry:
 
   board_autoled_off(LED_SIGNAL);
 #ifdef CONFIG_SMP
+  rtcb->irqcount++;
+  leave_critical_section(regs[REG_CPSR]);
   rtcb->irqcount--;
 #endif
   arm_fullcontextrestore(regs);

arch/arm64/src/common/arm64_sigdeliver.c

@@ -71,7 +71,6 @@ void arm64_sigdeliver(void)
   struct regs_context  *pctx =
                 (struct regs_context *)rtcb->xcp.saved_reg;
   flags = (pctx->spsr & SPSR_DAIF_MASK);
-  enter_critical_section();
 #endif
 
   sinfo("rtcb=%p sigdeliver=%p sigpendactionq.head=%p\n",
@@ -86,17 +85,16 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section(flags);
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -127,7 +125,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -140,6 +138,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section(flags);
+#endif
       goto retry;
     }
 

arch/risc-v/src/common/riscv_sigdeliver.c

@@ -64,8 +64,6 @@ void riscv_sigdeliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -82,17 +80,16 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section(regs[REG_INT_CTX]);
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -121,7 +118,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -134,6 +131,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section(regs[REG_INT_CTX]);
+#endif
       goto retry;
     }
 

arch/sparc/src/sparc_v8/sparc_v8_sigdeliver.c

@@ -72,8 +72,6 @@ void sparc_sigdeliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -94,17 +92,16 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need call leave_critical_section() repeatedly to get the irqcount
    * to zero, freeing all global spinlocks that enforce the critical section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section((regs[REG_PSR]));
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -150,6 +147,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section((regs[REG_PSR]));
+#endif
       goto retry;
     }
 

arch/xtensa/src/common/xtensa_sigdeliver.c

@@ -63,8 +63,6 @@ void xtensa_sig_deliver(void)
    */
 
   int16_t saved_irqcount;
-
-  enter_critical_section();
 #endif
 
   board_autoled_on(LED_SIGNAL);
@@ -81,18 +79,17 @@ retry:
    */
 
   saved_irqcount = rtcb->irqcount;
-  DEBUGASSERT(saved_irqcount >= 1);
+  DEBUGASSERT(saved_irqcount >= 0);
 
   /* Now we need to call leave_critical_section() repeatedly to get the
    * irqcount to zero, freeing all global spinlocks that enforce the critical
    * section.
    */
 
-  do
+  while (rtcb->irqcount > 0)
     {
       leave_critical_section((regs[REG_PS]));
     }
-  while (rtcb->irqcount > 0);
 #endif /* CONFIG_SMP */
 
 #ifndef CONFIG_SUPPRESS_INTERRUPTS
@@ -120,7 +117,7 @@ retry:
    */
 
   DEBUGASSERT(rtcb->irqcount == 0);
-  while (rtcb->irqcount < saved_irqcount)
+  while (rtcb->irqcount < saved_irqcount + 1)
     {
       enter_critical_section();
     }
@@ -133,6 +130,9 @@ retry:
   if (!sq_empty(&rtcb->sigpendactionq) &&
       (rtcb->flags & TCB_FLAG_SIGNAL_ACTION) == 0)
     {
+#ifdef CONFIG_SMP
+      leave_critical_section((regs[REG_PS]));
+#endif
       goto retry;
     }