Commit 2752d7c
test: check that sysconf returns a positive value
Static analysis insists that sysconf(_SC_PAGE_SIZE) might return a
negative integer (even though it never will). This was supposed to be
handled by the existing check EXPECT_GE(page, static_cast<int>(N)).
I assume that static analysis does not consider this sufficient because
static_cast<int>(N) could be negative or zero if N exceeds INT_MAX (even
though it never will).
To resolve this (theoretical) problem, explicitly check that the return
value is positive and then cast it to a size_t.
PR-URL: #44666
Reviewed-By: Darshan Sen <raisinten@gmail.com>1 parent c1d1ed5 commit 2752d7c
1 file changed
Lines changed: 22 additions & 11 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -29,25 +29,38 @@ | |||
| 29 | 29 | #endif | |
| 30 | 30 | #endif | |
| 31 | 31 | ||
| 32 | + #if defined(USE_MPROTECT) | ||
| 33 | + size_t GetPageSize() { | ||
| 34 | + int page_size = sysconf(_SC_PAGE_SIZE); | ||
| 35 | + EXPECT_GE(page_size, 1); | ||
| 36 | + return page_size; | ||
| 37 | + } | ||
| 38 | + #elif defined(USE_VIRTUALPROTECT) | ||
| 39 | + size_t GetPageSize() { | ||
| 40 | + SYSTEM_INFO system_info; | ||
| 41 | + GetSystemInfo(&system_info); | ||
| 42 | + return system_info.dwPageSize; | ||
| 43 | + } | ||
| 44 | + #endif | ||
| 45 | + | ||
| 32 | 46 | template <size_t N> | |
| 33 | 47 | class OverrunGuardedBuffer { | |
| 34 | 48 | public: | |
| 35 | 49 | OverrunGuardedBuffer() { | |
| 50 | + #if defined(USE_MPROTECT) || defined(USE_VIRTUALPROTECT) | ||
| 51 | + size_t page = GetPageSize(); | ||
| 52 | + EXPECT_GE(page, N); | ||
| 53 | + #endif | ||
| 36 | 54 | #ifdef USE_MPROTECT | |
| 37 | 55 | // Place the packet right before a guard page, which, when accessed, causes | |
| 38 | 56 | // a segmentation fault. | |
| 39 | - int page = sysconf(_SC_PAGE_SIZE); | ||
| 40 | - EXPECT_GE(page, static_cast<int>(N)); | ||
| 41 | 57 | alloc_base = static_cast<uint8_t*>(aligned_alloc(page, 2 * page)); | |
| 42 | 58 | EXPECT_NE(alloc_base, nullptr); | |
| 43 | 59 | uint8_t* second_page = alloc_base + page; | |
| 44 | 60 | EXPECT_EQ(mprotect(second_page, page, PROT_NONE), 0); | |
| 45 | 61 | data_base = second_page - N; | |
| 46 | 62 | #elif defined(USE_VIRTUALPROTECT) | |
| 47 | 63 | // On Windows, it works almost the same way. | |
| 48 | - SYSTEM_INFO system_info; | ||
| 49 | - GetSystemInfo(&system_info); | ||
| 50 | - DWORD page = system_info.dwPageSize; | ||
| 51 | 64 | alloc_base = static_cast<uint8_t*>( | |
| 52 | 65 | VirtualAlloc(nullptr, 2 * page, MEM_COMMIT, PAGE_READWRITE)); | |
| 53 | 66 | EXPECT_NE(alloc_base, nullptr); | |
@@ -70,16 +83,14 @@ class OverrunGuardedBuffer { | |||
| 70 | 83 | OverrunGuardedBuffer& operator=(const OverrunGuardedBuffer& other) = delete; | |
| 71 | 84 | ||
| 72 | 85 | ~OverrunGuardedBuffer() { | |
| 86 | + #if defined(USE_MPROTECT) || defined(USE_VIRTUALPROTECT) | ||
| 87 | + size_t page = GetPageSize(); | ||
| 88 | + #endif | ||
| 73 | 89 | #ifdef USE_VIRTUALPROTECT | |
| 74 | - SYSTEM_INFO system_info; | ||
| 75 | - GetSystemInfo(&system_info); | ||
| 76 | - DWORD page = system_info.dwPageSize; | ||
| 77 | - VirtualFree(alloc_base, 2 * system_info.dwPageSize, MEM_RELEASE); | ||
| 90 | + VirtualFree(alloc_base, 2 * page, MEM_RELEASE); | ||
| 78 | 91 | #else | |
| 79 | 92 | #ifdef USE_MPROTECT | |
| 80 | 93 | // Revert page protection such that the memory can be free()'d. | |
| 81 | - int page = sysconf(_SC_PAGE_SIZE); | ||
| 82 | - EXPECT_GE(page, static_cast<int>(N)); | ||
| 83 | 94 | uint8_t* second_page = alloc_base + page; | |
| 84 | 95 | EXPECT_EQ(mprotect(second_page, page, PROT_READ | PROT_WRITE), 0); | |
| 85 | 96 | #endif | |
0 commit comments