Skip to content

Commit

Permalink
MultiAddressSpaceLocker::AddAreaCacheAndLock(): race condition
Browse files Browse the repository at this point in the history
* 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.
  • Loading branch information
weinhold committed Jul 11, 2013
1 parent 7bea020 commit 02b151d
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
4 changes: 1 addition & 3 deletions src/system/kernel/vm/VMAddressSpaceLocking.cpp
Expand Up @@ -511,7 +511,7 @@ MultiAddressSpaceLocker::AddAreaCacheAndLock(area_id areaID,
VMArea* firstArea = cache->areas; VMArea* firstArea = cache->areas;
for (VMArea* current = firstArea; current; for (VMArea* current = firstArea; current;
current = current->cache_next) { current = current->cache_next) {
error = AddArea(current->id, error = AddArea(current,
current == area ? writeLockThisOne : writeLockOthers); current == area ? writeLockThisOne : writeLockOthers);
if (error != B_OK) { if (error != B_OK) {
vm_area_put_locked_cache(cache); vm_area_put_locked_cache(cache);
Expand Down Expand Up @@ -569,5 +569,3 @@ MultiAddressSpaceLocker::AddAreaCacheAndLock(area_id areaID,
memcpy(fItems, originalItems, fCount * sizeof(lock_item)); memcpy(fItems, originalItems, fCount * sizeof(lock_item));
} }
} }


11 changes: 11 additions & 0 deletions src/system/kernel/vm/VMAddressSpaceLocking.h
Expand Up @@ -91,6 +91,8 @@ class MultiAddressSpaceLocker : private AddressSpaceLockerBase {
VMAddressSpace** _space = NULL); VMAddressSpace** _space = NULL);
inline status_t AddArea(area_id area, bool writeLock, inline status_t AddArea(area_id area, bool writeLock,
VMAddressSpace** _space = NULL); VMAddressSpace** _space = NULL);
inline status_t AddArea(VMArea* area, bool writeLock,
VMAddressSpace** _space = NULL);


status_t AddAreaCacheAndLock(area_id areaID, status_t AddAreaCacheAndLock(area_id areaID,
bool writeLockThisOne, bool writeLockOthers, bool writeLockThisOne, bool writeLockOthers,
Expand Down Expand Up @@ -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 #endif // VM_ADDRESS_SPACE_LOCKING_H

0 comments on commit 02b151d

Please sign in to comment.