Skip to content

Commit

Permalink
test: check that sysconf returns a positive value
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
tniessen authored and juanarbol committed Oct 10, 2022
1 parent dae89c0 commit c017382
Showing 1 changed file with 22 additions and 11 deletions.
33 changes: 22 additions & 11 deletions test/cctest/test_crypto_clienthello.cc
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,38 @@
#endif
#endif

#if defined(USE_MPROTECT)
size_t GetPageSize() {
int page_size = sysconf(_SC_PAGE_SIZE);
EXPECT_GE(page_size, 1);
return page_size;
}
#elif defined(USE_VIRTUALPROTECT)
size_t GetPageSize() {
SYSTEM_INFO system_info;
GetSystemInfo(&system_info);
return system_info.dwPageSize;
}
#endif

template <size_t N>
class OverrunGuardedBuffer {
public:
OverrunGuardedBuffer() {
#if defined(USE_MPROTECT) || defined(USE_VIRTUALPROTECT)
size_t page = GetPageSize();
EXPECT_GE(page, N);
#endif
#ifdef USE_MPROTECT
// Place the packet right before a guard page, which, when accessed, causes
// a segmentation fault.
int page = sysconf(_SC_PAGE_SIZE);
EXPECT_GE(page, static_cast<int>(N));
alloc_base = static_cast<uint8_t*>(aligned_alloc(page, 2 * page));
EXPECT_NE(alloc_base, nullptr);
uint8_t* second_page = alloc_base + page;
EXPECT_EQ(mprotect(second_page, page, PROT_NONE), 0);
data_base = second_page - N;
#elif defined(USE_VIRTUALPROTECT)
// On Windows, it works almost the same way.
SYSTEM_INFO system_info;
GetSystemInfo(&system_info);
DWORD page = system_info.dwPageSize;
alloc_base = static_cast<uint8_t*>(
VirtualAlloc(nullptr, 2 * page, MEM_COMMIT, PAGE_READWRITE));
EXPECT_NE(alloc_base, nullptr);
Expand All @@ -70,16 +83,14 @@ class OverrunGuardedBuffer {
OverrunGuardedBuffer& operator=(const OverrunGuardedBuffer& other) = delete;

~OverrunGuardedBuffer() {
#if defined(USE_MPROTECT) || defined(USE_VIRTUALPROTECT)
size_t page = GetPageSize();
#endif
#ifdef USE_VIRTUALPROTECT
SYSTEM_INFO system_info;
GetSystemInfo(&system_info);
DWORD page = system_info.dwPageSize;
VirtualFree(alloc_base, 2 * system_info.dwPageSize, MEM_RELEASE);
VirtualFree(alloc_base, 2 * page, MEM_RELEASE);
#else
#ifdef USE_MPROTECT
// Revert page protection such that the memory can be free()'d.
int page = sysconf(_SC_PAGE_SIZE);
EXPECT_GE(page, static_cast<int>(N));
uint8_t* second_page = alloc_base + page;
EXPECT_EQ(mprotect(second_page, page, PROT_READ | PROT_WRITE), 0);
#endif
Expand Down

0 comments on commit c017382

Please sign in to comment.