[incubator-nuttx] branch master updated: sched: task: Fix a potential bug in nxtask_assign_pid()

[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/incubator-nuttx.git


The following commit(s) were added to refs/heads/master by this push:
     new e277ac7  sched: task: Fix a potential bug in nxtask_assign_pid()
e277ac7 is described below

commit e277ac7a7f32e3f115df28b4e8f4673de0cba87d
Author: Masayuki Ishikawa <ma...@gmail.com>
AuthorDate: Mon Jan 25 08:36:29 2021 +0900

    sched: task: Fix a potential bug in nxtask_assign_pid()
    
    Summary:
    - During reviewing sched_lock() in nxtask_assign_pid(),
      I noticed that g_pidhash is not protected by a critical section
    - Because g_pidhash is accessed in an interrupt context,
      it should be protected by a critical section.
    - Actually, nxsched_foreach(), nxsched_get_tcb() and so on
      use a critical section.
    
    Impact:
    - No impact
    
    Testing:
    - Tested with spresense:wifi (non-SMP) and spresense:wifi_smp
    
    Signed-off-by: Masayuki Ishikawa <Ma...@jp.sony.com>
---
 sched/task/task_setup.c | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/sched/task/task_setup.c b/sched/task/task_setup.c
index 363ac99..87e9124 100644
--- a/sched/task/task_setup.c
+++ b/sched/task/task_setup.c
@@ -81,12 +81,13 @@ static int nxtask_assign_pid(FAR struct tcb_s *tcb)
   pid_t next_pid;
   int   hash_ndx;
   int   tries;
+  int   ret = ERROR;
 
-  /* Disable pre-emption.  This should provide sufficient protection
-   * for the following operation.
+  /* Protect the following operation with a critical section
+   * because g_pidhash is accessed from an interrupt context
    */
 
-  sched_lock();
+  irqstate_t flags = enter_critical_section();
 
   /* We'll try every allowable pid */
 
@@ -121,17 +122,18 @@ static int nxtask_assign_pid(FAR struct tcb_s *tcb)
 #endif
           tcb->pid = next_pid;
 
-          sched_unlock();
-          return OK;
+          ret = OK;
+          goto errout;
         }
     }
 
+errout:
   /* If we get here, then the g_pidhash[] table is completely full.
    * We cannot allow another task to be started.
    */
 
-  sched_unlock();
-  return ERROR;
+  leave_critical_section(flags);
+  return ret;
 }
 
 /****************************************************************************