(nuttx) branch master updated: task/pthread_cancelpt: Fix task_delete from another task group

[xi...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

xiaoxiang pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new 57de6484e9 task/pthread_cancelpt: Fix task_delete from another task group
57de6484e9 is described below

commit 57de6484e9197fe8f50314fa3786f322bb70bac4
Author: Ville Juven <vi...@unikie.com>
AuthorDate: Tue Nov 21 14:56:36 2023 +0200

    task/pthread_cancelpt: Fix task_delete from another task group
    
    PR #11165 causes an unnecessary regression; task_delete no longer works,
    if the deleted task is from another group.
    
    The logic that prevents this comes from:
    
    nxnotify_cancellation() ->
    tls_get_info_pid() ->
    nxsched_get_stackinfo()
    
    Which checks for permissions, which does not make sense in this case since
    it is the kernel asking for the stack information.
    
    Fix this by partially reverting 11165 and implementing a direct path for
    the kernel to query for any tasks TLS.
---
 include/nuttx/tls.h                                | 21 +----------
 libs/libc/tls/tls_getinfo.c                        |  8 ++--
 sched/sched/CMakeLists.txt                         |  1 +
 sched/sched/Make.defs                              |  2 +-
 sched/sched/sched.h                                |  5 +++
 .../tls_getinfo.c => sched/sched/sched_get_tls.c   | 43 ++++++----------------
 sched/task/task_cancelpt.c                         |  2 +-
 7 files changed, 25 insertions(+), 57 deletions(-)

diff --git a/include/nuttx/tls.h b/include/nuttx/tls.h
index 967c6194bb..8b685a456c 100644
--- a/include/nuttx/tls.h
+++ b/include/nuttx/tls.h
@@ -316,28 +316,9 @@ uintptr_t task_tls_get_value(int tlsindex);
 #elif defined(CONFIG_TLS_ALIGNED) && !defined(__KERNEL__)
 #  define tls_get_info() TLS_INFO(up_getsp())
 #else
-#  define tls_get_info() tls_get_info_pid(0)
+FAR struct tls_info_s *tls_get_info(void);
 #endif
 
-/****************************************************************************
- * Name: tls_get_info_pid
- *
- * Description:
- *   Return a reference to the tls_info_s structure.  This is used as part
- *   of the internal implementation of tls_get/set_elem() and ONLY for the
- *   where CONFIG_TLS_ALIGNED is *not* defined or __KERNEL__ is defined.
- *
- * Input Parameters:
- *   pid - Thread ID to query, set to 0 to query own
- *
- * Returned Value:
- *   A reference to the thread-specific tls_info_s structure is return on
- *   success.  NULL would be returned in the event of any failure.
- *
- ****************************************************************************/
-
-FAR struct tls_info_s *tls_get_info_pid(pid_t pid);
-
 /****************************************************************************
  * Name: tls_destruct
  *
diff --git a/libs/libc/tls/tls_getinfo.c b/libs/libc/tls/tls_getinfo.c
index f613ea85a4..25ed601280 100644
--- a/libs/libc/tls/tls_getinfo.c
+++ b/libs/libc/tls/tls_getinfo.c
@@ -36,7 +36,7 @@
  ****************************************************************************/
 
 /****************************************************************************
- * Name: tls_get_info_pid
+ * Name: tls_get_info
  *
  * Description:
  *   Return a reference to the tls_info_s structure.  This is used as part
@@ -44,7 +44,7 @@
  *   where CONFIG_TLS_ALIGNED is *not* defined or __KERNEL__ is defined.
  *
  * Input Parameters:
- *   pid - Thread ID to query, set to 0 to query own
+ *   None.
  *
  * Returned Value:
  *   A reference to the thread-specific tls_info_s structure is return on
@@ -52,13 +52,13 @@
  *
  ****************************************************************************/
 
-FAR struct tls_info_s *tls_get_info_pid(pid_t pid)
+FAR struct tls_info_s *tls_get_info(void)
 {
   FAR struct tls_info_s *info = NULL;
   struct stackinfo_s stackinfo;
   int ret;
 
-  ret = nxsched_get_stackinfo(pid, &stackinfo);
+  ret = nxsched_get_stackinfo(0, &stackinfo);
   if (ret >= 0)
     {
       /* The TLS data lies at the lowest address of the stack allocation.
diff --git a/sched/sched/CMakeLists.txt b/sched/sched/CMakeLists.txt
index f7616d9ece..29ac1b6346 100644
--- a/sched/sched/CMakeLists.txt
+++ b/sched/sched/CMakeLists.txt
@@ -43,6 +43,7 @@ set(SRCS
     sched_idletask.c
     sched_self.c
     sched_get_stackinfo.c
+    sched_get_tls.c
     sched_sysinfo.c
     sched_reprioritizertr.c
     sched_get_stateinfo.c)
diff --git a/sched/sched/Make.defs b/sched/sched/Make.defs
index ddcb6c7580..422f010738 100644
--- a/sched/sched/Make.defs
+++ b/sched/sched/Make.defs
@@ -27,7 +27,7 @@ CSRCS += sched_setparam.c sched_setpriority.c sched_getparam.c
 CSRCS += sched_setscheduler.c sched_getscheduler.c
 CSRCS += sched_yield.c sched_rrgetinterval.c sched_foreach.c
 CSRCS += sched_lock.c sched_unlock.c sched_lockcount.c
-CSRCS += sched_idletask.c sched_self.c sched_get_stackinfo.c
+CSRCS += sched_idletask.c sched_self.c sched_get_stackinfo.c sched_get_tls.c
 CSRCS += sched_sysinfo.c sched_reprioritizertr.c sched_get_stateinfo.c
 
 ifeq ($(CONFIG_PRIORITY_INHERITANCE),y)
diff --git a/sched/sched/sched.h b/sched/sched/sched.h
index 79a5736b9b..104d19ed8e 100644
--- a/sched/sched/sched.h
+++ b/sched/sched/sched.h
@@ -417,4 +417,9 @@ void nxsched_suspend_critmon(FAR struct tcb_s *tcb);
 
 bool nxsched_verify_tcb(FAR struct tcb_s *tcb);
 
+/* Obtain TLS from kernel */
+
+struct tls_info_s; /* Forward declare */
+FAR struct tls_info_s *nxsched_get_tls(FAR struct tcb_s *tcb);
+
 #endif /* __SCHED_SCHED_SCHED_H */
diff --git a/libs/libc/tls/tls_getinfo.c b/sched/sched/sched_get_tls.c
similarity index 57%
copy from libs/libc/tls/tls_getinfo.c
copy to sched/sched/sched_get_tls.c
index f613ea85a4..ed8352ed89 100644
--- a/libs/libc/tls/tls_getinfo.c
+++ b/sched/sched/sched_get_tls.c
@@ -1,5 +1,5 @@
 /****************************************************************************
- * libs/libc/tls/tls_getinfo.c
+ * sched/sched/sched_get_tls.c
  *
  * Licensed to the Apache Software Foundation (ASF) under one or more
  * contributor license agreements.  See the NOTICE file distributed with
@@ -24,51 +24,32 @@
 
 #include <nuttx/config.h>
 
-#include <stdint.h>
-#include <assert.h>
-
-#include <nuttx/tls.h>
-
-#if !defined(up_tls_info) && (defined(__KERNEL__) || !defined(CONFIG_TLS_ALIGNED))
+#include "nuttx/sched.h"
+#include "sched/sched.h"
 
 /****************************************************************************
  * Public Functions
  ****************************************************************************/
 
 /****************************************************************************
- * Name: tls_get_info_pid
+ * Name: nxsched_get_tls
  *
  * Description:
- *   Return a reference to the tls_info_s structure.  This is used as part
- *   of the internal implementation of tls_get/set_elem() and ONLY for the
- *   where CONFIG_TLS_ALIGNED is *not* defined or __KERNEL__ is defined.
+ *   Get TLS of any task / tcb with no security checks.
  *
  * Input Parameters:
- *   pid - Thread ID to query, set to 0 to query own
+ *   tcb - The tcb to query.
  *
  * Returned Value:
- *   A reference to the thread-specific tls_info_s structure is return on
- *   success.  NULL would be returned in the event of any failure.
+ *   Pointer to the TLS structure.
  *
  ****************************************************************************/
 
-FAR struct tls_info_s *tls_get_info_pid(pid_t pid)
+FAR struct tls_info_s *nxsched_get_tls(FAR struct tcb_s *tcb)
 {
-  FAR struct tls_info_s *info = NULL;
-  struct stackinfo_s stackinfo;
-  int ret;
-
-  ret = nxsched_get_stackinfo(pid, &stackinfo);
-  if (ret >= 0)
-    {
-      /* The TLS data lies at the lowest address of the stack allocation.
-       * This is true for both push-up and push-down stacks.
-       */
+  /* The TLS data lies at the lowest address of the stack allocation.
+   * This is true for both push-up and push-down stacks.
+   */
 
-      info = (FAR struct tls_info_s *)stackinfo.stack_alloc_ptr;
-    }
-
-  return info;
+  return (FAR struct tls_info_s *)tcb->stack_alloc_ptr;
 }
-
-#endif /* !defined(up_tls_info) && (defined(__KERNEL__) || !defined(CONFIG_TLS_ALIGNED)) */
diff --git a/sched/task/task_cancelpt.c b/sched/task/task_cancelpt.c
index 4001152d18..834644fada 100644
--- a/sched/task/task_cancelpt.c
+++ b/sched/task/task_cancelpt.c
@@ -88,7 +88,7 @@
 
 bool nxnotify_cancellation(FAR struct tcb_s *tcb)
 {
-  FAR struct tls_info_s *tls = tls_get_info_pid(tcb->pid);
+  FAR struct tls_info_s *tls = nxsched_get_tls(tcb);
   irqstate_t flags;
   bool ret = false;