Fix flakes in tests that use munmap() to create deliberate MM holes.

PiperOrigin-RevId: 479623755
This commit is contained in:
Jamie Liu
2022-10-07 11:09:09 -07:00
committed by gVisor bot
parent c1e03c23aa
commit 543e8a2256
6 changed files with 94 additions and 64 deletions
+2
View File
@@ -1057,6 +1057,8 @@ cc_binary(
"@com_google_absl//absl/time",
gtest,
"//test/util:io_uring_util",
"//test/util:memory_util",
"//test/util:multiprocess_util",
"//test/util:temp_path",
"//test/util:test_main",
"//test/util:test_util",
+13 -3
View File
@@ -27,6 +27,8 @@
#include "gtest/gtest.h"
#include "test/util/io_uring_util.h"
#include "test/util/memory_util.h"
#include "test/util/multiprocess_util.h"
#include "test/util/test_util.h"
namespace gvisor {
@@ -72,10 +74,18 @@ TEST(IOUringTest, MMapMUnMapWork) {
EXPECT_NE(ptr, MAP_FAILED);
ASSERT_THAT(munmap(ptr, sring_sz), SyscallSucceeds());
const auto rest = [&] {
// N.B. we must be in a single-threaded subprocess to ensure that another
// thread doesn't racily remap at ptr.
TEST_PCHECK_MSG(MunmapSafe(ptr, sring_sz) == 0, "munmap failed");
// This should SIGSEGV.
*reinterpret_cast<volatile int *>(ptr) = 42;
};
EXPECT_EXIT(*reinterpret_cast<volatile int *>(ptr) = 42,
::testing::KilledBySignal(SIGSEGV), "");
int child_exit_status = ASSERT_NO_ERRNO_AND_VALUE(InForkedProcess(rest));
EXPECT_TRUE(WIFSIGNALED(child_exit_status) &&
WTERMSIG(child_exit_status) == SIGSEGV)
<< "exit status: " << child_exit_status;
}
// Testing that both mmap fails with EINVAL when an invalid offset is passed.
+5 -4
View File
@@ -92,7 +92,7 @@ class MMapTest : public ::testing::Test {
return -1;
}
int ret = munmap(addr_, length_);
int ret = MunmapSafe(addr_, length_);
addr_ = nullptr;
length_ = 0;
@@ -298,9 +298,10 @@ TEST_F(MMapTest, MapDevZeroSegfaultAfterUnmap) {
*reinterpret_cast<volatile int*>(addr_saved) = 0xFF;
};
EXPECT_THAT(InForkedProcess(rest),
IsPosixErrorOkAndHolds(AnyOf(Eq(W_EXITCODE(0, SIGSEGV)),
Eq(W_EXITCODE(0, 128 + SIGSEGV)))));
int child_exit_status = ASSERT_NO_ERRNO_AND_VALUE(InForkedProcess(rest));
EXPECT_TRUE(WIFSIGNALED(child_exit_status) &&
WTERMSIG(child_exit_status) == SIGSEGV)
<< "exit status: " << child_exit_status;
}
TEST_F(MMapTest, MapDevZeroUnaligned) {
+33 -33
View File
@@ -42,8 +42,8 @@ namespace {
// libc mremap isn't guaranteed to be async-signal-safe by signal-safety(7) and
// therefore isn't necessarily safe to call between fork(2) and execve(2);
// provide our own version that is.
void* safe_mremap(void* old_addr, size_t old_size, size_t new_size,
unsigned long flags, void* new_address = nullptr) { // NOLINT
void* MremapSafe(void* old_addr, size_t old_size, size_t new_size,
unsigned long flags, void* new_address = nullptr) { // NOLINT
return reinterpret_cast<void*>(
syscall(SYS_mremap, old_addr, old_size, new_size, flags, new_address));
}
@@ -67,7 +67,7 @@ TEST_P(MremapParamTest, InPlace_ShrinkingWholeVMA) {
const auto rest = [&] {
// N.B. we must be in a single-threaded subprocess to ensure a
// background thread doesn't concurrently map the second page.
void* addr = safe_mremap(m.ptr(), 2 * kPageSize, kPageSize, 0, nullptr);
void* addr = MremapSafe(m.ptr(), 2 * kPageSize, kPageSize, 0, nullptr);
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == m.ptr());
MaybeSave();
@@ -84,7 +84,7 @@ TEST_P(MremapParamTest, InPlace_ShrinkingPartialVMA) {
ASSERT_NO_ERRNO_AND_VALUE(MmapAnon(3 * kPageSize, PROT_NONE, GetParam()));
const auto rest = [&] {
void* addr = safe_mremap(m.ptr(), 2 * kPageSize, kPageSize, 0, nullptr);
void* addr = MremapSafe(m.ptr(), 2 * kPageSize, kPageSize, 0, nullptr);
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == m.ptr());
MaybeSave();
@@ -106,7 +106,7 @@ TEST_P(MremapParamTest, InPlace_ShrinkingAcrossVMAs) {
const auto rest = [&] {
// Both old_size and new_size now span two vmas; mremap
// shouldn't care.
void* addr = safe_mremap(m.ptr(), 3 * kPageSize, 2 * kPageSize, 0, nullptr);
void* addr = MremapSafe(m.ptr(), 3 * kPageSize, 2 * kPageSize, 0, nullptr);
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == m.ptr());
MaybeSave();
@@ -128,11 +128,11 @@ TEST_P(MremapParamTest, InPlace_ExpansionSuccess) {
//
// N.B. we must be in a single-threaded subprocess to ensure a
// background thread doesn't concurrently map this page.
TEST_PCHECK(
munmap(reinterpret_cast<void*>(m.addr() + kPageSize), kPageSize) == 0);
TEST_PCHECK(MunmapSafe(reinterpret_cast<void*>(m.addr() + kPageSize),
kPageSize) == 0);
MaybeSave();
void* addr = safe_mremap(m.ptr(), kPageSize, 2 * kPageSize, 0, nullptr);
void* addr = MremapSafe(m.ptr(), kPageSize, 2 * kPageSize, 0, nullptr);
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == m.ptr());
MaybeSave();
@@ -152,11 +152,11 @@ TEST_P(MremapParamTest, InPlace_ExpansionFailure) {
// Unmap the second page, leaving a one-page hole. Trying to expand the
// first page to three pages should fail since the original third page
// is still mapped.
TEST_PCHECK(
munmap(reinterpret_cast<void*>(m.addr() + kPageSize), kPageSize) == 0);
TEST_PCHECK(MunmapSafe(reinterpret_cast<void*>(m.addr() + kPageSize),
kPageSize) == 0);
MaybeSave();
void* addr = safe_mremap(m.ptr(), kPageSize, 3 * kPageSize, 0, nullptr);
void* addr = MremapSafe(m.ptr(), kPageSize, 3 * kPageSize, 0, nullptr);
TEST_CHECK_MSG(addr == MAP_FAILED, "mremap unexpectedly succeeded");
TEST_PCHECK_MSG(errno == ENOMEM, "mremap failed with wrong errno");
MaybeSave();
@@ -178,12 +178,12 @@ TEST_P(MremapParamTest, MayMove_Expansion) {
// first page to three pages with MREMAP_MAYMOVE should force the
// mapping to be relocated since the original third page is still
// mapped.
TEST_PCHECK(
munmap(reinterpret_cast<void*>(m.addr() + kPageSize), kPageSize) == 0);
TEST_PCHECK(MunmapSafe(reinterpret_cast<void*>(m.addr() + kPageSize),
kPageSize) == 0);
MaybeSave();
void* addr2 =
safe_mremap(m.ptr(), kPageSize, 3 * kPageSize, MREMAP_MAYMOVE, nullptr);
MremapSafe(m.ptr(), kPageSize, 3 * kPageSize, MREMAP_MAYMOVE, nullptr);
TEST_PCHECK_MSG(addr2 != MAP_FAILED, "mremap failed");
MaybeSave();
@@ -219,11 +219,11 @@ TEST_P(MremapParamTest, Fixed_SameSize) {
const auto rest = [&] {
// Unmap dst to create a hole.
TEST_PCHECK(munmap(dst.ptr(), kPageSize) == 0);
TEST_PCHECK(MunmapSafe(dst.ptr(), kPageSize) == 0);
MaybeSave();
void* addr = safe_mremap(src.ptr(), kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
void* addr = MremapSafe(src.ptr(), kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == dst.ptr());
MaybeSave();
@@ -244,8 +244,8 @@ TEST_P(MremapParamTest, Fixed_SameSize_Unmapping) {
ASSERT_NO_ERRNO_AND_VALUE(MmapAnon(kPageSize, PROT_NONE, GetParam()));
const auto rest = [&] {
void* addr = safe_mremap(src.ptr(), kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
void* addr = MremapSafe(src.ptr(), kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == dst.ptr());
MaybeSave();
@@ -266,11 +266,11 @@ TEST_P(MremapParamTest, Fixed_ShrinkingWholeVMA) {
const auto rest = [&] {
// Unmap dst so we can check that mremap does not keep the
// second page.
TEST_PCHECK(munmap(dst.ptr(), 2 * kPageSize) == 0);
TEST_PCHECK(MunmapSafe(dst.ptr(), 2 * kPageSize) == 0);
MaybeSave();
void* addr = safe_mremap(src.ptr(), 2 * kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
void* addr = MremapSafe(src.ptr(), 2 * kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == dst.ptr());
MaybeSave();
@@ -293,11 +293,11 @@ TEST_P(MremapParamTest, Fixed_ShrinkingPartialVMA) {
const auto rest = [&] {
// Unmap dst so we can check that mremap does not keep the
// second page.
TEST_PCHECK(munmap(dst.ptr(), 2 * kPageSize) == 0);
TEST_PCHECK(MunmapSafe(dst.ptr(), 2 * kPageSize) == 0);
MaybeSave();
void* addr = safe_mremap(src.ptr(), 2 * kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
void* addr = MremapSafe(src.ptr(), 2 * kPageSize, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == dst.ptr());
MaybeSave();
@@ -323,8 +323,8 @@ TEST_P(MremapParamTest, Fixed_ShrinkingAcrossVMAs) {
const auto rest = [&] {
// Unlike flags=0, MREMAP_FIXED requires that [old_address,
// old_address+new_size) only spans a single vma.
void* addr = safe_mremap(src.ptr(), 3 * kPageSize, 2 * kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
void* addr = MremapSafe(src.ptr(), 3 * kPageSize, 2 * kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
TEST_CHECK_MSG(addr == MAP_FAILED, "mremap unexpectedly succeeded");
TEST_PCHECK_MSG(errno == EFAULT, "mremap failed with wrong errno");
MaybeSave();
@@ -351,11 +351,11 @@ TEST_P(MremapParamTest, Fixed_Expansion) {
const auto rest = [&] {
// Unmap dst so we can check that mremap actually maps all pages
// at the destination.
TEST_PCHECK(munmap(dst.ptr(), 2 * kPageSize) == 0);
TEST_PCHECK(MunmapSafe(dst.ptr(), 2 * kPageSize) == 0);
MaybeSave();
void* addr = safe_mremap(src.ptr(), kPageSize, 2 * kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
void* addr = MremapSafe(src.ptr(), kPageSize, 2 * kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
TEST_PCHECK_MSG(addr != MAP_FAILED, "mremap failed");
TEST_CHECK(addr == dst.ptr());
MaybeSave();
@@ -389,7 +389,7 @@ TEST(MremapTest, MayMove_Copy) {
// Remainder of this test executes in a subprocess to ensure that if mremap
// incorrectly removes m, it is not remapped by another thread.
const auto rest = [&] {
void* ptr = safe_mremap(m.ptr(), 0, kPageSize, MREMAP_MAYMOVE, nullptr);
void* ptr = MremapSafe(m.ptr(), 0, kPageSize, MREMAP_MAYMOVE, nullptr);
MaybeSave();
TEST_PCHECK_MSG(ptr != MAP_FAILED, "mremap failed");
TEST_CHECK(ptr != m.ptr());
@@ -408,8 +408,8 @@ TEST(MremapTest, MustMove_Copy) {
// Remainder of this test executes in a subprocess to ensure that if mremap
// incorrectly removes src, it is not remapped by another thread.
const auto rest = [&] {
void* ptr = safe_mremap(src.ptr(), 0, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
void* ptr = MremapSafe(src.ptr(), 0, kPageSize,
MREMAP_MAYMOVE | MREMAP_FIXED, dst.ptr());
MaybeSave();
TEST_PCHECK_MSG(ptr != MAP_FAILED, "mremap failed");
TEST_CHECK(ptr == dst.ptr());
+33 -23
View File
@@ -563,13 +563,20 @@ TEST(ProcPidMem, Unmapped) {
SyscallSucceedsWithValue(sizeof(output)));
ASSERT_EQ(expected, output);
// Unmap region again
ASSERT_THAT(munmap(mapping.ptr(), mapping.len()), SyscallSucceeds());
const auto rest = [&] {
// This is a new process, so we need to re-open /proc/self/mem.
int memfd = open("/proc/self/mem", O_RDONLY);
TEST_PCHECK_MSG(memfd >= 0, "open failed");
// Unmap region again
TEST_PCHECK_MSG(MunmapSafe(mapping.ptr(), mapping.len()) == 0,
"munmap failed");
// Now we want EIO error
TEST_CHECK(pread(memfd, &output, sizeof(output),
reinterpret_cast<off_t>(mapping.ptr())) == -1);
TEST_PCHECK_MSG(errno == EIO, "pread failed with unexpected errno");
};
// Now we want EIO error
ASSERT_THAT(pread(memfd.get(), &output, sizeof(output),
reinterpret_cast<off_t>(mapping.ptr())),
SyscallFailsWithErrno(EIO));
EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0));
}
// Perform read repeatedly to verify offset change.
@@ -623,29 +630,32 @@ TEST(ProcPidMem, RepeatedSeek) {
// Perform read past an allocated memory region.
TEST(ProcPidMem, PartialRead) {
// Strategy: map large region, then do unmap and remap smaller region
auto memfd = ASSERT_NO_ERRNO_AND_VALUE(Open("/proc/self/mem", O_RDONLY));
// Reserve 2 pages.
Mapping mapping = ASSERT_NO_ERRNO_AND_VALUE(
MmapAnon(2 * kPageSize, PROT_READ | PROT_WRITE, MAP_PRIVATE));
ASSERT_THAT(munmap(mapping.ptr(), mapping.len()), SyscallSucceeds());
Mapping smaller_mapping = ASSERT_NO_ERRNO_AND_VALUE(
Mmap(mapping.ptr(), kPageSize, PROT_READ | PROT_WRITE,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0));
// Fill it with things
memset(smaller_mapping.ptr(), 'x', smaller_mapping.len());
// Fill the first page with data.
memset(mapping.ptr(), 'x', kPageSize);
// Now we want no error
char expected[] = {'x'};
std::unique_ptr<char[]> output(new char[kPageSize]);
off_t read_offset =
reinterpret_cast<off_t>(smaller_mapping.ptr()) + kPageSize - 1;
ASSERT_THAT(
pread(memfd.get(), output.get(), sizeof(output.get()), read_offset),
SyscallSucceedsWithValue(sizeof(expected)));
// Since output is larger, than expected we have to do manual compare
ASSERT_EQ(expected[0], (output).get()[0]);
off_t read_offset = reinterpret_cast<off_t>(mapping.ptr()) + kPageSize - 1;
const auto rest = [&] {
int memfd = open("/proc/self/mem", O_RDONLY);
TEST_PCHECK_MSG(memfd >= 0, "open failed");
// Unmap the second page.
TEST_PCHECK_MSG(
MunmapSafe(reinterpret_cast<void*>(mapping.addr() + kPageSize),
kPageSize) == 0,
"munmap failed");
// Expect to read up to the end of the first page without getting EIO.
TEST_PCHECK_MSG(
pread(memfd, output.get(), kPageSize, read_offset) == sizeof(expected),
"pread failed");
TEST_CHECK(expected[0] == output.get()[0]);
};
EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0));
}
// Perform read on /proc/[pid]/mem after exit.
+8 -1
View File
@@ -19,6 +19,8 @@
#include <stddef.h>
#include <stdint.h>
#include <sys/mman.h>
#include <sys/syscall.h>
#include <unistd.h>
#include "absl/strings/str_format.h"
#include "absl/strings/string_view.h"
@@ -30,6 +32,11 @@
namespace gvisor {
namespace testing {
// Async-signal-safe version of munmap(2).
inline int MunmapSafe(void* addr, size_t length) {
return syscall(SYS_munmap, addr, length);
}
// RAII type for mmap'ed memory. Only usable in tests due to use of a test-only
// macro that can't be named without invoking the presubmit's wrath.
class Mapping {
@@ -82,7 +89,7 @@ class Mapping {
void reset(void* ptr, size_t len) {
if (len_) {
TEST_PCHECK(munmap(ptr_, len_) == 0);
TEST_PCHECK(MunmapSafe(ptr_, len_) == 0);
}
ptr_ = ptr;
len_ = len;