From 02b151d3e344772bcc6f91c874b9b87a23350a0a Mon Sep 17 00:00:00 2001 From: Ingo Weinhold Date: Thu, 11 Jul 2013 17:54:55 +0200 Subject: [PATCH] MultiAddressSpaceLocker::AddAreaCacheAndLock(): race condition * Add a VMArea* version of AddArea(). * AddAreaCacheAndLock(): Use the new AddArea() version. This not only saves the ID hash table lookup, but also fixes a race condition with delete_area(). delete_area() removes the area from the hash before removing it from its cache, so iterating through the cache's areas can turn up an area that no longer is in the hash. In that case we would fail immediately. The new AddArea() won't fail in this situation, though. Fixes #9686: vm_copy_area() could fail for the "commpage" area. That's an area all teams share, so any team terminating while another one was fork()ing could trigger it. --- src/system/kernel/vm/VMAddressSpaceLocking.cpp | 4 +--- src/system/kernel/vm/VMAddressSpaceLocking.h | 11 +++++++++++ 2 files changed, 12 insertions(+), 3 deletions(-) diff --git a/src/system/kernel/vm/VMAddressSpaceLocking.cpp b/src/system/kernel/vm/VMAddressSpaceLocking.cpp index 0fb7da4436..d1bc1c3247 100644 --- a/src/system/kernel/vm/VMAddressSpaceLocking.cpp +++ b/src/system/kernel/vm/VMAddressSpaceLocking.cpp @@ -511,7 +511,7 @@ MultiAddressSpaceLocker::AddAreaCacheAndLock(area_id areaID, VMArea* firstArea = cache->areas; for (VMArea* current = firstArea; current; current = current->cache_next) { - error = AddArea(current->id, + error = AddArea(current, current == area ? writeLockThisOne : writeLockOthers); if (error != B_OK) { vm_area_put_locked_cache(cache); @@ -569,5 +569,3 @@ MultiAddressSpaceLocker::AddAreaCacheAndLock(area_id areaID, memcpy(fItems, originalItems, fCount * sizeof(lock_item)); } } - - diff --git a/src/system/kernel/vm/VMAddressSpaceLocking.h b/src/system/kernel/vm/VMAddressSpaceLocking.h index cf2ed7d62f..34ec33a8d9 100644 --- a/src/system/kernel/vm/VMAddressSpaceLocking.h +++ b/src/system/kernel/vm/VMAddressSpaceLocking.h @@ -91,6 +91,8 @@ public: VMAddressSpace** _space = NULL); inline status_t AddArea(area_id area, bool writeLock, VMAddressSpace** _space = NULL); + inline status_t AddArea(VMArea* area, bool writeLock, + VMAddressSpace** _space = NULL); status_t AddAreaCacheAndLock(area_id areaID, bool writeLockThisOne, bool writeLockOthers, @@ -139,4 +141,13 @@ MultiAddressSpaceLocker::AddArea(area_id area, bool writeLock, } +inline status_t +MultiAddressSpaceLocker::AddArea(VMArea* area, bool writeLock, + VMAddressSpace** _space) +{ + area->address_space->Get(); + return _AddAddressSpace(area->address_space, writeLock, _space); +} + + #endif // VM_ADDRESS_SPACE_LOCKING_H