Fix out-of-bounds write in case of failing mmap(...) in PosixLockedPageAllocator::AllocateLocked

practicalswift · practicalswift · commit ca126d490b0f · 2019-01-06T17:45:53.000+01:00
diff --git a/src/support/allocators/secure.h b/src/support/allocators/secure.h
@@ -40,7 +40,11 @@ struct secure_allocator : public std::allocator<T> {
 
     T* allocate(std::size_t n, const void* hint = 0)
     {
-        return static_cast<T*>(LockedPoolManager::Instance().alloc(sizeof(T) * n));
+        T* allocation = static_cast<T*>(LockedPoolManager::Instance().alloc(sizeof(T) * n));
+        if (!allocation) {
+            throw std::bad_alloc();
+        }
+        return allocation;
     }
 
     void deallocate(T* p, std::size_t n)
diff --git a/src/support/lockedpool.cpp b/src/support/lockedpool.cpp
@@ -248,6 +248,9 @@ void *PosixLockedPageAllocator::AllocateLocked(size_t len, bool *lockingSuccess)
     void *addr;
     len = align_up(len, page_size);
     addr = mmap(nullptr, len, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0);
+    if (addr == MAP_FAILED) {
+        return nullptr;
+    }
     if (addr) {
         *lockingSuccess = mlock(addr, len) == 0;
     }
diff --git a/src/support/lockedpool.h b/src/support/lockedpool.h
@@ -22,7 +22,7 @@ class LockedPageAllocator
     virtual ~LockedPageAllocator() {}
     /** Allocate and lock memory pages.
      * If len is not a multiple of the system page size, it is rounded up.
-     * Returns 0 in case of allocation failure.
+     * Returns nullptr in case of allocation failure.
      *
      * If locking the memory pages could not be accomplished it will still
      * return the memory, however the lockingSuccess flag will be false.

Original file line number	Diff line number	Diff line change
`@@ -40,7 +40,11 @@ struct secure_allocator : public std::allocator<T> {`
`40`	`40`
`41`	`41`	`T* allocate(std::size_t n, const void* hint = 0)`
`42`	`42`	`{`
`43`		`- return static_cast<T>(LockedPoolManager::Instance().alloc(sizeof(T) n));`
	`43`	`+ T* allocation = static_cast<T>(LockedPoolManager::Instance().alloc(sizeof(T) n));`
	`44`	`+ if (!allocation) {`
	`45`	`+ throw std::bad_alloc();`
	`46`	`+ }`
	`47`	`+ return allocation;`
`44`	`48`	`}`
`45`	`49`
`46`	`50`	`void deallocate(T* p, std::size_t n)`
Original file line number	Diff line number	Diff line change
`@@ -22,7 +22,7 @@ class LockedPageAllocator`
`22`	`22`	`virtual ~LockedPageAllocator() {}`
`23`	`23`	`/** Allocate and lock memory pages.`
`24`	`24`	`* If len is not a multiple of the system page size, it is rounded up.`
`25`		`- * Returns 0 in case of allocation failure.`
	`25`	`+ * Returns nullptr in case of allocation failure.`
`26`	`26`	`*`
`27`	`27`	`* If locking the memory pages could not be accomplished it will still`
`28`	`28`	`* return the memory, however the lockingSuccess flag will be false.`