diff options
author | Jamie Liu <jamieliu@google.com> | 2019-01-11 14:47:45 -0800 |
---|---|---|
committer | Shentubot <shentubot@google.com> | 2019-01-11 14:49:39 -0800 |
commit | bf65e06c5f00eb41e40dfbb07dda31c6b7ae443e (patch) | |
tree | 4265fa80597165f24456e109053636fad637c10f /test/syscalls/linux | |
parent | 290bcb6de9c4aeff65bbfa06b8addecdbc51ca88 (diff) |
Clean up some uses of fork() in tests.
- Fix a few cases where async-signal-unsafe code is executed in a forked
process pre-execve.
- Ensure that the return value of fork() is always checked.
PiperOrigin-RevId: 228949310
Change-Id: I3096cb7d7394b8d9ab81b0e0245f2060713ef589
Diffstat (limited to 'test/syscalls/linux')
-rw-r--r-- | test/syscalls/linux/BUILD | 1 | ||||
-rw-r--r-- | test/syscalls/linux/concurrency.cc | 8 | ||||
-rw-r--r-- | test/syscalls/linux/kill.cc | 19 | ||||
-rw-r--r-- | test/syscalls/linux/ptrace.cc | 65 | ||||
-rw-r--r-- | test/syscalls/linux/sync_file_range.cc | 1 | ||||
-rw-r--r-- | test/syscalls/linux/timers.cc | 3 | ||||
-rw-r--r-- | test/syscalls/linux/wait.cc | 1 |
7 files changed, 66 insertions, 32 deletions
diff --git a/test/syscalls/linux/BUILD b/test/syscalls/linux/BUILD index a8a6e15ee..41d476481 100644 --- a/test/syscalls/linux/BUILD +++ b/test/syscalls/linux/BUILD @@ -1460,7 +1460,6 @@ cc_binary( "//test/util:logging", "//test/util:multiprocess_util", "//test/util:signal_util", - "//test/util:test_main", "//test/util:test_util", "//test/util:thread_util", "@com_google_absl//absl/time", diff --git a/test/syscalls/linux/concurrency.cc b/test/syscalls/linux/concurrency.cc index 2c13b315c..f5a941ca8 100644 --- a/test/syscalls/linux/concurrency.cc +++ b/test/syscalls/linux/concurrency.cc @@ -72,8 +72,7 @@ TEST(ConcurrencyTest, MultiProcessMultithreaded) { } }); - pid_t child_pid; - ASSERT_THAT(child_pid = fork(), SyscallSucceeds()); + pid_t child_pid = fork(); if (child_pid == 0) { // Busy wait without making any blocking syscalls. auto end = absl::Now() + absl::Seconds(5); @@ -81,6 +80,7 @@ TEST(ConcurrencyTest, MultiProcessMultithreaded) { } _exit(0); } + ASSERT_THAT(child_pid, SyscallSucceeds()); absl::SleepFor(absl::Seconds(1)); @@ -99,13 +99,13 @@ TEST(ConcurrencyTest, MultiProcessMultithreaded) { // never yields. TEST(ConcurrencyTest, MultiProcessConcurrency) { - pid_t child_pid; - ASSERT_THAT(child_pid = fork(), SyscallSucceeds()); + pid_t child_pid = fork(); if (child_pid == 0) { while (true) { } __builtin_unreachable(); } + ASSERT_THAT(child_pid, SyscallSucceeds()); absl::SleepFor(absl::Seconds(5)); diff --git a/test/syscalls/linux/kill.cc b/test/syscalls/linux/kill.cc index 18ba8fb16..de50daf0b 100644 --- a/test/syscalls/linux/kill.cc +++ b/test/syscalls/linux/kill.cc @@ -70,7 +70,7 @@ TEST(KillTest, CanKillAllPIDs) { sa.sa_sigaction = SigHandler; sigfillset(&sa.sa_mask); sa.sa_flags = SA_SIGINFO; - auto cleanup = ASSERT_NO_ERRNO_AND_VALUE(ScopedSigaction(SIGWINCH, sa)); + auto cleanup = ScopedSigaction(SIGWINCH, sa).ValueOrDie(); // Indicate to the parent that we're ready. write_fd.reset(); @@ -81,7 +81,7 @@ TEST(KillTest, CanKillAllPIDs) { } } - EXPECT_THAT(pid, SyscallSucceeds()); + ASSERT_THAT(pid, SyscallSucceeds()); write_fd.reset(); @@ -111,7 +111,7 @@ TEST(KillTest, CannotKillInvalidPID) { _exit(0); } - EXPECT_THAT(fake_pid, SyscallSucceeds()); + ASSERT_THAT(fake_pid, SyscallSucceeds()); int status; ASSERT_THAT(RetryEINTR(waitpid)(fake_pid, &status, 0), @@ -134,7 +134,7 @@ TEST(KillTest, CanKillRemoteProcess) { } } - EXPECT_THAT(pid, SyscallSucceeds()); + ASSERT_THAT(pid, SyscallSucceeds()); EXPECT_THAT(kill(pid, SIGKILL), SyscallSucceeds()); @@ -182,7 +182,7 @@ TEST(KillTest, SetPgid) { } } - EXPECT_THAT(pid, SyscallSucceeds()); + ASSERT_THAT(pid, SyscallSucceeds()); // Set the child's group and exit. ASSERT_THAT(setpgid(pid, pid), SyscallSucceeds()); @@ -208,7 +208,7 @@ TEST(KillTest, ProcessGroups) { pause(); } } - EXPECT_THAT(child, SyscallSucceeds()); + ASSERT_THAT(child, SyscallSucceeds()); pid_t other_child = fork(); if (other_child == 0) { @@ -216,6 +216,7 @@ TEST(KillTest, ProcessGroups) { pause(); } } + ASSERT_THAT(other_child, SyscallSucceeds()); // Ensure the kill does not succeed without the new group. EXPECT_THAT(kill(-child, SIGKILL), SyscallFailsWithErrno(ESRCH)); @@ -298,7 +299,7 @@ TEST(KillTest, ChildDropsPrivsCannotKill) { _exit(0); } - EXPECT_THAT(pid, SyscallSucceeds()); + ASSERT_THAT(pid, SyscallSucceeds()); int status; EXPECT_THAT(RetryEINTR(waitpid)(pid, &status, 0), @@ -316,7 +317,7 @@ TEST(KillTest, CanSIGCONTSameSession) { _exit(0); } - EXPECT_THAT(stopped_child, SyscallSucceeds()); + ASSERT_THAT(stopped_child, SyscallSucceeds()); // Put the child in its own process group. The child and parent process // groups also share a session. @@ -359,7 +360,7 @@ TEST(KillTest, CanSIGCONTSameSession) { _exit(0); } - EXPECT_THAT(stopped_child, SyscallSucceeds()); + ASSERT_THAT(stopped_child, SyscallSucceeds()); // Make sure child exited normally. EXPECT_THAT(RetryEINTR(waitpid)(stopped_child, &status, 0), diff --git a/test/syscalls/linux/ptrace.cc b/test/syscalls/linux/ptrace.cc index d3b3b8b02..f063cc92b 100644 --- a/test/syscalls/linux/ptrace.cc +++ b/test/syscalls/linux/ptrace.cc @@ -34,6 +34,11 @@ #include "test/util/test_util.h" #include "test/util/thread_util.h" +DEFINE_bool(ptrace_test_execve_child, false, + "If true, run the " + "PtraceExecveTest_Execve_GetRegs_PeekUser_SIGKILL_TraceClone_" + "TraceExit child workload."); + namespace gvisor { namespace testing { @@ -457,26 +462,18 @@ class PtraceExecveTest : public ::testing::TestWithParam<bool> { }; TEST_P(PtraceExecveTest, Execve_GetRegs_PeekUser_SIGKILL_TraceClone_TraceExit) { + ExecveArray const owned_child_argv = {"/proc/self/exe", + "--ptrace_test_execve_child"}; + char* const* const child_argv = owned_child_argv.get(); + pid_t const child_pid = fork(); if (child_pid == 0) { - // In child process. - - // Enable tracing, then raise SIGSTOP and expect our parent to suppress it. - TEST_PCHECK(ptrace(PTRACE_TRACEME, 0, 0, 0) == 0); - MaybeSave(); - RaiseSignal(SIGSTOP); - MaybeSave(); - - // Call execve in a non-leader thread. - ExecveArray const owned_child_argv = {"/proc/self/exe"}; - char* const* const child_argv = owned_child_argv.get(); - ScopedThread t([&] { - execve(child_argv[0], child_argv, /* envp = */ nullptr); - TEST_CHECK_MSG(false, "Survived execve? (thread)"); - }); - t.Join(); - TEST_CHECK_MSG(false, "Survived execve? (main)"); - _exit(1); + // In child process. The test relies on calling execve() in a non-leader + // thread; pthread_create() isn't async-signal-safe, so the safest way to + // do this is to execve() first, then enable tracing and run the expected + // child process behavior in the new subprocess. + execve(child_argv[0], child_argv, /* envp = */ nullptr); + TEST_PCHECK_MSG(false, "Survived execve to test child"); } // In parent process. ASSERT_THAT(child_pid, SyscallSucceeds()); @@ -595,6 +592,28 @@ TEST_P(PtraceExecveTest, Execve_GetRegs_PeekUser_SIGKILL_TraceClone_TraceExit) { << " status " << status; } +[[noreturn]] void RunExecveChild() { + // Enable tracing, then raise SIGSTOP and expect our parent to suppress it. + TEST_PCHECK(ptrace(PTRACE_TRACEME, 0, 0, 0) == 0); + MaybeSave(); + RaiseSignal(SIGSTOP); + MaybeSave(); + + // Call execve() in a non-leader thread. As long as execve() succeeds, what + // exactly we execve() shouldn't really matter, since the tracer should kill + // us after execve() completes. + ScopedThread t([&] { + ExecveArray const owned_child_argv = {"/proc/self/exe", + "--this_flag_shouldnt_exist"}; + char* const* const child_argv = owned_child_argv.get(); + execve(child_argv[0], child_argv, /* envp = */ nullptr); + TEST_PCHECK_MSG(false, "Survived execve? (thread)"); + }); + t.Join(); + TEST_CHECK_MSG(false, "Survived execve? (main)"); + _exit(1); +} + INSTANTIATE_TEST_CASE_P(TraceExec, PtraceExecveTest, ::testing::Bool()); // This test has expectations on when syscall-enter/exit-stops occur that are @@ -946,3 +965,13 @@ TEST(PtraceTest, ERESTART_NoRandomSave) { } // namespace testing } // namespace gvisor + +int main(int argc, char** argv) { + gvisor::testing::TestInit(&argc, &argv); + + if (FLAGS_ptrace_test_execve_child) { + gvisor::testing::RunExecveChild(); + } + + return RUN_ALL_TESTS(); +} diff --git a/test/syscalls/linux/sync_file_range.cc b/test/syscalls/linux/sync_file_range.cc index ebe4ca171..d11f58481 100644 --- a/test/syscalls/linux/sync_file_range.cc +++ b/test/syscalls/linux/sync_file_range.cc @@ -70,6 +70,7 @@ TEST(SyncFileRangeTest, CannotSyncFileRangeOnUnopenedFd) { TEST_PCHECK(errno == EBADF); _exit(0); } + ASSERT_THAT(pid, SyscallSucceeds()); int status = 0; ASSERT_THAT(waitpid(pid, &status, 0), SyscallSucceedsWithValue(pid)); diff --git a/test/syscalls/linux/timers.cc b/test/syscalls/linux/timers.cc index dfe231575..14506eb12 100644 --- a/test/syscalls/linux/timers.cc +++ b/test/syscalls/linux/timers.cc @@ -92,6 +92,7 @@ TEST(TimerTest, ProcessKilledOnCPUSoftLimit) { for (;;) { } } + ASSERT_THAT(pid, SyscallSucceeds()); auto c = Cleanup([pid] { int status; EXPECT_THAT(waitpid(pid, &status, 0), SyscallSucceedsWithValue(pid)); @@ -150,6 +151,7 @@ TEST(TimerTest, ProcessPingedRepeatedlyAfterCPUSoftLimit) { for (;;) { } } + ASSERT_THAT(pid, SyscallSucceeds()); auto c = Cleanup([pid] { int status; EXPECT_THAT(waitpid(pid, &status, 0), SyscallSucceedsWithValue(pid)); @@ -195,6 +197,7 @@ TEST(TimerTest, ProcessKilledOnCPUHardLimit) { for (;;) { } } + ASSERT_THAT(pid, SyscallSucceeds()); auto c = Cleanup([pid] { int status; EXPECT_THAT(waitpid(pid, &status, 0), SyscallSucceedsWithValue(pid)); diff --git a/test/syscalls/linux/wait.cc b/test/syscalls/linux/wait.cc index 0a4ec7c6a..2da0bf130 100644 --- a/test/syscalls/linux/wait.cc +++ b/test/syscalls/linux/wait.cc @@ -547,6 +547,7 @@ TEST_P(WaitSpecificChildTest, AfterChildExecve) { const_cast<char**>(child_argv)); _exit(errno); } + ASSERT_THAT(child, SyscallSucceeds()); EXPECT_NO_ERRNO(WaitFor(child, 0)); } |