From 252f58f6e9c7e9640f8a04d9380db99fd5b2ccec Mon Sep 17 00:00:00 2001 From: Gregory Nutt Date: Sun, 3 May 2020 17:15:15 -0600 Subject: [PATCH] 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 c00a5b825e..8d076011ed 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 4803fb7fa2..2ce750268d 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(stackinfo != NULL); + DEBUGASSERT(rtcb != NULL && 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; }