Skip to content

GitLab

T
tangled
Commit 82c3f702 authored by Robert Sesek's avatar Robert Sesek Committed by Commit Bot
Browse files
Options

Simplify the ParameterRestrictions.ptrace* tests. 

These were disabled because they were flaky on 32-bit kernels and some
Linux environments (like Chromecast). Rather than setting up the complex
process hierarchy to have a real ptracer/tracee relationship, just issue
the ptrace calls and test for the right error and/or death.

Bug: 934930
Change-Id: Ib2d97ab25e7931d2b5ba8bd607f9db36184eb4d5
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1856798
Commit-Queue: Robert Sesek <rsesek@chromium.org>
Reviewed-by: default avatarJorge Lucangeli Obes <jorgelo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#705578}
parent fa7fdde8
Hide whitespace changes
Inline Side-by-side
Showing with 33 additions and 199 deletions
sandbox/linux/seccomp-bpf-helpers/syscall_parameters_restrictions_unittests.cc
View file @ 82c3f702
...@@ -248,15 +248,9 @@ BPF_DEATH_TEST_C(ParameterRestrictions, ...@@ -248,15 +248,9 @@ BPF_DEATH_TEST_C(ParameterRestrictions,
getrusage(RUSAGE_CHILDREN, &usage); getrusage(RUSAGE_CHILDREN, &usage);
} }
// ptace() Tests /////////////////////////////////////////////////////////////// // The following ptrace() tests do not actually set up a tracer/tracee
// relationship, so allowed operations return ESRCH errors. Blocked operations
// Tests for ptrace involve a slightly complex setup in order to properly test // are tested with death tests.
// ptrace and the variety of ways it is access-checked. The BPF_TEST macro,
// the body of which already runs in its own process, spawns another process
// called the "tracee". The "tracee" then spawns another process called the
// "tracer". The child then traces the parent and performs the test operations.
// The tracee must be careful to un-stop the tracer if the tracee expects to
// die.
class RestrictPtracePolicy : public bpf_dsl::Policy { class RestrictPtracePolicy : public bpf_dsl::Policy {
public: public:
...@@ -273,208 +267,48 @@ class RestrictPtracePolicy : public bpf_dsl::Policy { ...@@ -273,208 +267,48 @@ class RestrictPtracePolicy : public bpf_dsl::Policy {
} }
}; };
constexpr char kExitPtraceChildClean = '!';
class PtraceTestHarness {
public:
using PtraceChildTracerFunc = void (*)(pid_t tracee);
PtraceTestHarness(PtraceChildTracerFunc tracer_func, bool expect_death)
: tracer_func_(tracer_func), expect_death_(expect_death) {}
~PtraceTestHarness() = default;
void Run() {
// Fork the tracee process that will be traced by its child.
pid_t pid = fork();
BPF_ASSERT_GE(pid, 0);
if (pid == 0) {
RunTracee();
} else {
// The tracee should always exit cleanly.
int status = 0;
int rv = waitpid(pid, &status, 0);
BPF_ASSERT_EQ(pid, rv);
BPF_ASSERT_EQ(0, WEXITSTATUS(status));
}
}
private:
void RunTracee() {
// Create a communications pipe between tracer and tracee.
int rv = pipe2(pipes_, O_NONBLOCK);
BPF_ASSERT_EQ(0, rv);
// Pipes for redirecting output.
int output_pipes[2];
BPF_ASSERT_EQ(0, pipe(output_pipes));
// Create the tracer process.
pid_t pid = fork();
BPF_ASSERT_GE(pid, 0);
if (pid == 0) {
// Close the pipe read ends and redirect output.
close(pipes_[0]);
close(output_pipes[0]);
close(STDOUT_FILENO);
dup2(output_pipes[1], STDOUT_FILENO);
close(STDERR_FILENO);
dup2(output_pipes[1], STDERR_FILENO);
RunTracer();
close(output_pipes[1]);
} else {
close(pipes_[1]);
close(output_pipes[1]);
// Ensure the tracer can trace the tracee. This may fail on systems
// without YAMA, so the result is not checked.
prctl(PR_SET_PTRACER, pid);
char c = 0;
while (c != kExitPtraceChildClean) {
// Read from the control channel in a non-blocking fashion.
// If no data are present, loop.
ignore_result(read(pipes_[0], &c, 1));
// Poll the exit status of the child.
int status = 0;
rv = waitpid(pid, &status, WNOHANG);
if (rv != 0) {
BPF_ASSERT_EQ(pid, rv);
CheckTracerStatus(status, output_pipes[0]);
_exit(0);
}
}
_exit(0);
}
}
void RunTracer() {
pid_t ppid = getppid();
BPF_ASSERT_NE(0, ppid);
// Attach to the tracee and then call out to the test function.
BPF_ASSERT_EQ(0, ptrace(PTRACE_ATTACH, ppid, nullptr, nullptr));
tracer_func_(ppid);
BPF_ASSERT_EQ(1, HANDLE_EINTR(write(pipes_[1], &kExitPtraceChildClean, 1)));
close(pipes_[1]);
_exit(0);
}
void CheckTracerStatus(int status, int output_pipe) {
// The child has exited. Test that it did so in the way we were
// expecting.
if (expect_death_) {
// This duplicates a bit of what //sandbox/linux/tests/unit_tests.cc does
// but that code is not shareable here.
std::string output;
const size_t kBufferSize = 1024;
size_t total_bytes_read = 0;
ssize_t read_this_pass = 0;
do {
output.resize(output.size() + kBufferSize);
read_this_pass = HANDLE_EINTR(
read(output_pipe, &output[total_bytes_read], kBufferSize));
if (read_this_pass >= 0) {
total_bytes_read += read_this_pass;
output.resize(total_bytes_read);
}
} while (read_this_pass > 0);
#if !defined(SANDBOX_USES_BASE_TEST_SUITE)
const bool subprocess_got_sigsegv =
WIFSIGNALED(status) && (SIGSEGV == WTERMSIG(status));
#else
// This hack is required when a signal handler is installed
// for SEGV that will _exit(1).
const bool subprocess_got_sigsegv =
WIFEXITED(status) && (1 == WEXITSTATUS(status));
#endif
BPF_ASSERT(subprocess_got_sigsegv);
BPF_ASSERT_NE(output.find(GetPtraceErrorMessageContentForTests()),
std::string::npos);
} else {
BPF_ASSERT(WIFEXITED(status));
BPF_ASSERT_EQ(0, WEXITSTATUS(status));
}
}
PtraceChildTracerFunc tracer_func_;
bool expect_death_;
int pipes_[2];
DISALLOW_COPY_AND_ASSIGN(PtraceTestHarness);
};
// Fails on Android L and M.
// See https://crbug.com/934930
BPF_TEST_C(ParameterRestrictions, BPF_TEST_C(ParameterRestrictions,
DISABLED_ptrace_getregs_allowed, ptrace_getregs_allowed,
RestrictPtracePolicy) { RestrictPtracePolicy) {
auto tracer = [](pid_t pid) {
#if defined(__arm__) #if defined(__arm__)
user_regs regs; user_regs regs;
#else #else
user_regs_struct regs; user_regs_struct regs;
#endif #endif
iovec iov; iovec iov;
iov.iov_base = &regs; iov.iov_base = &regs;
iov.iov_len = sizeof(regs); iov.iov_len = sizeof(regs);
BPF_ASSERT_EQ(0, ptrace(PTRACE_GETREGSET, pid, errno = 0;
reinterpret_cast<void*>(NT_PRSTATUS), &iov)); int rv = ptrace(PTRACE_GETREGSET, getpid(),
reinterpret_cast<void*>(NT_PRSTATUS), &iov);
BPF_ASSERT_EQ(0, ptrace(PTRACE_DETACH, pid, nullptr, nullptr)); BPF_ASSERT_EQ(-1, rv);
}; BPF_ASSERT_EQ(ESRCH, errno);
PtraceTestHarness(tracer, false).Run();
} }
// Fails on Android L and M. BPF_DEATH_TEST_C(
// See https://crbug.com/934930 ParameterRestrictions,
BPF_TEST_C(ParameterRestrictions, ptrace_syscall_blocked,
DISABLED_ptrace_syscall_blocked, DEATH_SEGV_MESSAGE(sandbox::GetPtraceErrorMessageContentForTests()),
RestrictPtracePolicy) { RestrictPtracePolicy) {
auto tracer = [](pid_t pid) { ptrace(PTRACE_SYSCALL, getpid(), nullptr, nullptr);
// The tracer is about to die. Make sure the tracee is not stopped so it
// can reap it and inspect its death signal.
kill(pid, SIGCONT);
BPF_ASSERT_NE(0, ptrace(PTRACE_SYSCALL, 0, nullptr, nullptr));
};
PtraceTestHarness(tracer, true).Run();
} }
BPF_TEST_C(ParameterRestrictions, BPF_DEATH_TEST_C(
DISABLED_ptrace_setregs_blocked, ParameterRestrictions,
RestrictPtracePolicy) { ptrace_setregs_blocked,
auto tracer = [](pid_t pid) { DEATH_SEGV_MESSAGE(sandbox::GetPtraceErrorMessageContentForTests()),
RestrictPtracePolicy) {
#if defined(__arm__) #if defined(__arm__)
user_regs regs; user_regs regs{};
#else #else
user_regs_struct regs; user_regs_struct regs{};
#endif #endif
iovec iov; iovec iov;
iov.iov_base = &regs; iov.iov_base = &regs;
iov.iov_len = sizeof(regs); iov.iov_len = sizeof(regs);
BPF_ASSERT_EQ(0, ptrace(PTRACE_GETREGSET, pid, errno = 0;
reinterpret_cast<void*>(NT_PRSTATUS), &iov)); ptrace(PTRACE_SETREGSET, getpid(), reinterpret_cast<void*>(NT_PRSTATUS),
&iov);
// The tracer is about to die. Make sure the tracee is not stopped so it
// can reap it and inspect its death signal.
kill(pid, SIGCONT);
BPF_ASSERT_NE(0, ptrace(PTRACE_SETREGSET, pid,
reinterpret_cast<void*>(NT_PRSTATUS), &iov));
};
PtraceTestHarness(tracer, true).Run();
} }
} // namespace } // namespace
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment