From e277ac7a7f32e3f115df28b4e8f4673de0cba87d Mon Sep 17 00:00:00 2001 From: Masayuki Ishikawa Date: Mon, 25 Jan 2021 08:36:29 +0900 Subject: [PATCH] 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 --- 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 363ac99d27c..87e9124528b 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; } /****************************************************************************