[incubator-nuttx] branch master updated: sched/sched/sched_get_stackinfo.c: Add some security.

[ag...@apache.org]

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

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


The following commit(s) were added to refs/heads/master by this push:
     new 252f58f  sched/sched/sched_get_stackinfo.c:  Add some security.
252f58f is described below

commit 252f58f6e9c7e9640f8a04d9380db99fd5b2ccec
Author: Gregory Nutt <gn...@nuttx.org>
AuthorDate: Sun May 3 17:15:15 2020 -0600

    sched/sched/sched_get_stackinfo.c:  Add some security.
    
    The sched_get_stackinfo() interface was just added.  However, it occurs to me that it is a dangerous feature and could lead to security problems.  In FLAT and PROTECTED modes, if you get access to any other threads stack, you could do harm.
    
    This commit adds some level of security.  Basically, it implements these rules:
    
    1. Any thread may query its own stack,
    2. A kernel thread may query the stack of any other thread
    3. Application threads, however, may query only the stacks of threads within the same task group, i.e., the main thread and any of the child pthreads created with the main thread as a parent or grandparent or great-grandpart ...
---
 include/nuttx/sched.h             |  6 ++++-
 sched/sched/sched_get_stackinfo.c | 48 +++++++++++++++++++++++++++++++--------
 2 files changed, 43 insertions(+), 11 deletions(-)

diff --git a/include/nuttx/sched.h b/include/nuttx/sched.h
index c00a5b8..8d07601 100644
--- a/include/nuttx/sched.h
+++ b/include/nuttx/sched.h
@@ -1245,13 +1245,17 @@ int nxsched_setaffinity(pid_t pid, size_t cpusetsize,
  *   Report information about a thread's stack allocation.
  *
  * Input Parameters:
- *   pid_t     - Identifies the thread to query.  Zero is interpreted as the
+ *   pid       - Identifies the thread to query.  Zero is interpreted as the
  *               the calling thread
  *   stackinfo - User-provided location to return the stack information.
  *
  * Returned Value:
  *   Zero (OK) if successful.  Otherwise, a negated errno value is returned.
  *
+ *     -ENOENT  Returned if pid does not refer to an active task
+ *     -EACCES  The calling thread does not have privileges to access the
+ *              stack of the thread associated with the pid.
+ *
  ********************************************************************************/
 
 int sched_get_stackinfo(pid_t pid, FAR struct stackinfo_s *stackinfo);
diff --git a/sched/sched/sched_get_stackinfo.c b/sched/sched/sched_get_stackinfo.c
index 4803fb7..2ce7502 100644
--- a/sched/sched/sched_get_stackinfo.c
+++ b/sched/sched/sched_get_stackinfo.c
@@ -41,37 +41,65 @@
  *   Report information about a thread's stack allocation.
  *
  * Input Parameters:
- *   pid_t     - Identifies the thread to query.  Zero is interpreted as the
+ *   pid       - Identifies the thread to query.  Zero is interpreted as the
  *               the calling thread
  *   stackinfo - User-provided location to return the stack information.
  *
  * Returned Value:
  *   Zero (OK) if successful.  Otherwise, a negated errno value is returned.
  *
+ *     -ENOENT  Returned if pid does not refer to an active task
+ *     -EACCES  The calling thread does not have privileges to access the
+ *              stack of the thread associated with the pid.
+ *
  ****************************************************************************/
 
 int sched_get_stackinfo(pid_t pid, FAR struct stackinfo_s *stackinfo)
 {
-  FAR struct tcb_s *tcb;
+  FAR struct tcb_s *rtcb = this_task();  /* TCB of running task */
+  FAR struct tcb_s *qtcb;                /* TCB of queried task */
+
+  DEBUGASSERT(rtcb != NULL && stackinfo != NULL);
 
-  DEBUGASSERT(stackinfo != NULL);
+  /*  Pid of 0 means that we are querying ourself */
 
   if (pid == 0)
     {
-      tcb = this_task();
-      DEBUGASSERT(tcb != NULL);
+      /* We can always query ourself */
+
+      qtcb = rtcb;
     }
   else
     {
-      tcb = sched_gettcb(pid);
-      if (tcb == NULL)
+      /* Get the task to be queried */
+
+      qtcb = sched_gettcb(pid);
+      if (qtcb == NULL)
         {
           return -ENOENT;
         }
+
+      /* A kernel thread can query any other thread.  Application threads
+       * can only query application threads in the same task group.
+       */
+
+      if ((rtcb->flags & TCB_FLAG_TTYPE_MASK) != TCB_FLAG_TTYPE_KERNEL)
+        {
+          /* It is an application thread.  It is permitted to query
+           * only threads within the same task group.  It is not permitted
+           * to peek into the stacks of either kernel threads or other
+           * applications tasks.
+           */
+
+          if (rtcb->group != qtcb->group)
+            {
+              return -EACCES;
+            }
+        }
     }
 
-  stackinfo->adj_stack_size  = tcb->adj_stack_size;
-  stackinfo->stack_alloc_ptr = tcb->stack_alloc_ptr;
-  stackinfo->adj_stack_ptr   = tcb->adj_stack_ptr;
+  stackinfo->adj_stack_size  = qtcb->adj_stack_size;
+  stackinfo->stack_alloc_ptr = qtcb->stack_alloc_ptr;
+  stackinfo->adj_stack_ptr   = qtcb->adj_stack_ptr;
   return OK;
 }