[haiku-commits] haiku: hrev53341 - in src/system/kernel: . debug

  • From: waddlesplash <waddlesplash@xxxxxxxxx>
  • To: haiku-commits@xxxxxxxxxxxxx
  • Date: Sun, 4 Aug 2019 20:26:20 -0400 (EDT)

hrev53341 adds 2 changesets to branch 'master'
old head: 8dae411c1efbf28fbd76c3ed0db66ffec475f2dc
new head: 719cfad7fd6da76610f7869f9ef93828804a083f
overview: 
https://git.haiku-os.org/haiku/log/?qt=range&q=719cfad7fd6d+%5E8dae411c1efb

----------------------------------------------------------------------------

9dc67a1d961a: kernel/condition_variable: Rework remove-from-condition logic 
again.
  
  This avoids a few potential race conditions mmlr pointed out on
  the mailing list. See inline comments.
  
  Change-Id: I605523c1d2683c749751599c417a68a20c70edea

719cfad7fd6d: kernel/user_debugger: Don't try to cast an integer to a pointer.
  
  Should fix the x86_64 build.

                              [ Augustin Cavalier <waddlesplash@xxxxxxxxx> ]

----------------------------------------------------------------------------

2 files changed, 22 insertions(+), 10 deletions(-)
src/system/kernel/condition_variable.cpp  | 28 +++++++++++++++++++--------
src/system/kernel/debug/user_debugger.cpp |  4 ++--

############################################################################

Commit:      9dc67a1d961a2a9d7dea3df8969071f27350a0ef
URL:         https://git.haiku-os.org/haiku/commit/?id=9dc67a1d961a
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Mon Aug  5 00:10:15 2019 UTC

kernel/condition_variable: Rework remove-from-condition logic again.

This avoids a few potential race conditions mmlr pointed out on
the mailing list. See inline comments.

Change-Id: I605523c1d2683c749751599c417a68a20c70edea

----------------------------------------------------------------------------

diff --git a/src/system/kernel/condition_variable.cpp 
b/src/system/kernel/condition_variable.cpp
index c33882e329..db8bf76f63 100644
--- a/src/system/kernel/condition_variable.cpp
+++ b/src/system/kernel/condition_variable.cpp
@@ -18,6 +18,7 @@
 #include <scheduling_analysis.h>
 #include <thread.h>
 #include <util/AutoLock.h>
+#include <util/atomic.h>
 
 
 #define STATUS_ADDED   1
@@ -143,14 +144,11 @@ ConditionVariableEntry::Wait(uint32 flags, bigtime_t 
timeout)
        if (fVariable == NULL)
                return fWaitStatus;
 
-       SpinLocker conditionLocker(fVariable->fLock);
-
        thread_prepare_to_block(fThread, flags,
                THREAD_BLOCK_TYPE_CONDITION_VARIABLE, fVariable);
 
        fWaitStatus = STATUS_WAITING;
 
-       conditionLocker.Unlock();
        entryLocker.Unlock();
 
        status_t error;
@@ -161,13 +159,27 @@ ConditionVariableEntry::Wait(uint32 flags, bigtime_t 
timeout)
 
        entryLocker.Lock();
 
-       // remove entry from variable, if not done yet
+       // Remove entry from variable, if not done yet. Since we are going to
+       // lock ourselves and recheck fVariable if it is not NULL, we don't need
+       // to acquire our own lock before doing this first NULL check.
        if (fVariable != NULL) {
-               conditionLocker.Lock();
-               // we need to check we are still in the variable's entries; 
there may
-               // have been a race for our removal.
-               if (fVariable->fEntries.Contains(this))
+               if (fVariable == NULL)
+                       return error;
+               SpinLocker conditionLocker(fVariable->fLock);
+               entryLocker.Unlock();
+
+               if (fVariable->fEntries.Contains(this)) {
                        fVariable->fEntries.Remove(this);
+               } else {
+                       // The variable's fEntries did not contain us, but we 
currently
+                       // have the variable's lock acquired. This must mean we 
are in
+                       // a race with the variable's Notify. It is possible we 
will be
+                       // destroyed immediately upon returning here, so we 
need to
+                       // spin until our fVariable member is unset by the 
Notify thread
+                       // and then re-acquire our own lock to avoid a 
use-after-free.
+                       while (atomic_pointer_get(&fVariable) != NULL) {}
+               }
+               entryLocker.Lock();
                fVariable = NULL;
        }
 

############################################################################

Revision:    hrev53341
Commit:      719cfad7fd6da76610f7869f9ef93828804a083f
URL:         https://git.haiku-os.org/haiku/commit/?id=719cfad7fd6d
Author:      Augustin Cavalier <waddlesplash@xxxxxxxxx>
Date:        Mon Aug  5 00:12:53 2019 UTC

kernel/user_debugger: Don't try to cast an integer to a pointer.

Should fix the x86_64 build.

----------------------------------------------------------------------------

diff --git a/src/system/kernel/debug/user_debugger.cpp 
b/src/system/kernel/debug/user_debugger.cpp
index e762e36d45..20eee0e027 100644
--- a/src/system/kernel/debug/user_debugger.cpp
+++ b/src/system/kernel/debug/user_debugger.cpp
@@ -2547,7 +2547,7 @@ install_team_debugger(team_id teamID, port_id 
debuggerPort,
        // get the team
        Team* team;
        ConditionVariable debugChangeCondition;
-       debugChangeCondition.Init((void*)teamID, "debug change condition");
+       debugChangeCondition.Init(NULL, "debug change condition");
        error = prepare_debugger_change(teamID, debugChangeCondition, team);
        if (error != B_OK)
                return error;
@@ -2874,7 +2874,7 @@ _user_remove_team_debugger(team_id teamID)
 {
        Team* team;
        ConditionVariable debugChangeCondition;
-       debugChangeCondition.Init((void*)teamID, "debug change condition");
+       debugChangeCondition.Init(NULL, "debug change condition");
        status_t error = prepare_debugger_change(teamID, debugChangeCondition,
                team);
        if (error != B_OK)


Other related posts:

  • » [haiku-commits] haiku: hrev53341 - in src/system/kernel: . debug - waddlesplash