[mynewt-core] branch master updated: kernel/os: Fix possible race in os_mutex_release()

[an...@apache.org]

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

andk pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mynewt-core.git


The following commit(s) were added to refs/heads/master by this push:
     new f4182fe  kernel/os: Fix possible race in os_mutex_release()
f4182fe is described below

commit f4182fe3557f16e2a2963974add370f0e1c97afb
Author: Andrzej Kaczmarek <an...@codecoup.pl>
AuthorDate: Wed Jan 30 14:55:03 2019 +0100

    kernel/os: Fix possible race in os_mutex_release()
    
    There is possible race between os_mutex_release() and os_mutex_pend()
    due to modification of mu_level outside critical section. The scenario
    is as follows:
    
    1. os_mutex_release() decrements mu_level on mutex which is not nested
       this means that now mu_level==0 and will be unlocked
    2. current task is preempted by another one which calls os_mutex_pend()
       on the same mutex
    3. os_mutex_pend() checks if mu_level==0 (i.e. unlocked) and locks it
       by setting mu_level=1 and mu_owner=<task>
    4. previous task resumes execution and enters critical section which
       should unlock the mutex and overwrites mu_owner=NULL (assuming there
       is no other task waiting to lock this mutex)
    5. the result is that mutex has mu_level>0 so it is assumed locked, but
       at the same time mu_owner==NULL which will cause hardfault when
       trying to lock it again due to NULL pointer access when comparing
       task priorities
    
    The solution here is to modify os_mutex_release() to check if mutex is
    nested with mu_level==1 (i.e. last lock) and if it is, remove last
    reference inside critical section.
---
 kernel/os/src/os_mutex.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/kernel/os/src/os_mutex.c b/kernel/os/src/os_mutex.c
index 5d1f880..3044e95 100644
--- a/kernel/os/src/os_mutex.c
+++ b/kernel/os/src/os_mutex.c
@@ -79,15 +79,18 @@ os_mutex_release(struct os_mutex *mu)
         goto done;
     }
 
-    /* Decrement nesting level by 1. If not zero, nested (so dont release!) */
-    --mu->mu_level;
-    if (mu->mu_level != 0) {
+    /* Don't release if nested, just decrement nesting level */
+    if (mu->mu_level != 1) {
+        --mu->mu_level;
         ret = OS_OK;
         goto done;
     }
 
     OS_ENTER_CRITICAL(sr);
 
+    /* Decrement nesting level (this effectively sets nesting level to 0) */
+    --mu->mu_level;
+
     /* Restore owner task's priority; resort list if different  */
     if (current->t_prio != mu->mu_prio) {
         current->t_prio = mu->mu_prio;