From 2cdba4a0a280d03d93a598da28407cce5c80f3d9 Mon Sep 17 00:00:00 2001
From: Ville Juven <ville.juven@unikie.com>
Date: Thu, 16 Feb 2023 10:13:27 +0200
Subject: [PATCH] task/task_cancelpt: Kill the child if it is not in a cancel
 point

Do not allow a deferred cancellation if the group is exiting, it is too
dangerous to allow the threads to execute any user space code after the
exit has started.

If the cancelled thread is not inside a cancellation point, just kill it
immediately via asynchronous cancellation. This will create far less
problems than allowing it to continue running user code.
---
 include/nuttx/sched.h            |  1 +
 sched/group/group_killchildren.c |  8 ++++++++
 sched/task/task_cancelpt.c       | 21 +++++++++++++++------
 3 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h
index 703ea64f5c..8eae2680bd 100644
--- a/include/nuttx/sched.h
+++ b/include/nuttx/sched.h
@@ -110,6 +110,7 @@
 #define GROUP_FLAG_NOCLDWAIT       (1 << 0)                      /* Bit 0: Do not retain child exit status */
 #define GROUP_FLAG_PRIVILEGED      (1 << 1)                      /* Bit 1: Group is privileged */
 #define GROUP_FLAG_DELETED         (1 << 2)                      /* Bit 2: Group has been deleted but not yet freed */
+#define GROUP_FLAG_EXITING         (1 << 3)                      /* Bit 3: Group exit is in progress */
                                                                  /* Bits 3-7: Available */
 
 /* Values for struct child_status_s ch_flags */
diff --git a/sched/group/group_killchildren.c b/sched/group/group_killchildren.c
index 4f35c6a38b..fa5a98d65c 100644
--- a/sched/group/group_killchildren.c
+++ b/sched/group/group_killchildren.c
@@ -132,6 +132,8 @@ int group_kill_children(FAR struct tcb_s *tcb)
 {
   int ret;
 
+  DEBUGASSERT(tcb->group);
+
 #ifdef CONFIG_SMP
   /* NOTE: sched_lock() is not enough for SMP
    * because tcb->group will be accessed from the child tasks
@@ -145,8 +147,14 @@ int group_kill_children(FAR struct tcb_s *tcb)
 
   sched_lock();
 #endif
+
+  /* Tell the children that this group has started exiting */
+
+  tcb->group->tg_flags |= GROUP_FLAG_EXITING;
+
   ret = group_foreachchild(tcb->group, group_kill_children_handler,
                            (FAR void *)((uintptr_t)tcb->pid));
+
 #ifdef CONFIG_SMP
   leave_critical_section(flags);
 #else
diff --git a/sched/task/task_cancelpt.c b/sched/task/task_cancelpt.c
index 0c8609f474..2a1ab912f8 100644
--- a/sched/task/task_cancelpt.c
+++ b/sched/task/task_cancelpt.c
@@ -323,6 +323,7 @@ bool check_cancellation_point(void)
 bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 {
   irqstate_t flags;
+  bool ret = false;
 
   /* We need perform the following operations from within a critical section
    * because it can compete with interrupt level activity.
@@ -362,9 +363,11 @@ bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 
   if ((tcb->flags & TCB_FLAG_CANCEL_DEFERRED) != 0)
     {
-      /* Then we cannot cancel the task asynchronously.
-       * Mark the cancellation as pending.
-       */
+      /* Then we cannot cancel the task asynchronously. */
+
+      ret = true;
+
+      /* Mark the cancellation as pending. */
 
       tcb->flags |= TCB_FLAG_CANCEL_PENDING;
 
@@ -405,11 +408,17 @@ bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 #endif
         }
 
-      leave_critical_section(flags);
-      return true;
+#ifdef HAVE_GROUP_MEMBERS
+      else if (tcb->group && (tcb->group->tg_flags & GROUP_FLAG_EXITING))
+        {
+          /* Exit in progress, do asynchronous cancel instead */
+
+          ret = false;
+        }
+#endif
     }
 #endif
 
   leave_critical_section(flags);
-  return false;
+  return ret;
 }