netbird/gvisor
0
0
Fork 0
mirror of https://github.com/netbirdio/gvisor.git synced 2026-05-22 17:12:49 -07:00
Code Issues Packages Projects Releases Wiki Activity

Fix async-signal-unsafety in chroot test.

Browse Source 
PiperOrigin-RevId: 384295543
This commit is contained in:
Jamie Liu
2021-07-12 12:49:48 -07:00
committed by gVisor bot
parent 4742f7d788
commit 9c09db654e
2 changed files with 142 additions and 79 deletions
Download Patch File Download Diff File Expand all files Collapse all files
test/syscalls/linux/BUILD
+1
View File
@@ -479,6 +479,7 @@ cc_binary(
"//test/util:cleanup",
"//test/util:file_descriptor",
"//test/util:fs_util",
"@com_google_absl//absl/cleanup",
"@com_google_absl//absl/strings",
gtest,
"//test/util:logging",
test/syscalls/linux/chroot.cc
+141 -79
View File
@@ -20,16 +20,17 @@
#include <syscall.h>
#include <unistd.h>
#include <algorithm>
#include <string>
#include <vector>
#include "gmock/gmock.h"
#include "gtest/gtest.h"
#include "absl/cleanup/cleanup.h"
#include "absl/strings/str_cat.h"
#include "absl/strings/str_split.h"
#include "absl/strings/string_view.h"
#include "test/util/capability_util.h"
#include "test/util/cleanup.h"
#include "test/util/file_descriptor.h"
#include "test/util/fs_util.h"
#include "test/util/logging.h"
@@ -46,13 +47,52 @@ namespace testing {
namespace {
// Async-signal-safe conversion from integer to string, appending the string
// (including a terminating NUL) to buf, which is a buffer of size len bytes.
// Returns the number of bytes written, or 0 if the buffer is too small.
//
// Preconditions: 2 <= radix <= 16.
template <typename T>
size_t SafeItoa(T val, char* buf, size_t len, int radix) {
size_t n = 0;
#define _WRITE_OR_FAIL(c) \
do { \
if (len == 0) { \
return 0; \
} \
buf[n] = (c); \
n++; \
len--; \
} while (false)
if (val == 0) {
_WRITE_OR_FAIL('0');
} else {
// Write digits in reverse order, then reverse them at the end.
bool neg = val < 0;
while (val != 0) {
// C/C++ define modulo such that the result is negative if exactly one of
// the dividend or divisor is negative, so this handles both positive and
// negative values.
char c = "fedcba9876543210123456789abcdef"[val % radix + 15];
_WRITE_OR_FAIL(c);
val /= 10;
}
if (neg) {
_WRITE_OR_FAIL('-');
}
std::reverse(buf, buf + n);
}
_WRITE_OR_FAIL('\0');
return n;
#undef _WRITE_OR_FAIL
}
TEST(ChrootTest, Success) {
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_CHROOT)));
auto temp_dir = TempPath::CreateDir().ValueOrDie();
const std::string temp_dir_path = temp_dir.path();
const auto rest = [] {
auto temp_dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
TEST_CHECK_SUCCESS(chroot(temp_dir.path().c_str()));
};
const auto rest = [&] { TEST_CHECK_SUCCESS(chroot(temp_dir_path.c_str())); };
EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0));
}
@@ -101,28 +141,34 @@ TEST(ChrootTest, CreatesNewRoot) {
SyscallSucceeds());
auto new_root = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const std::string new_root_path = new_root.path();
auto file_in_new_root =
ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFileIn(new_root.path()));
const std::string file_in_new_root_path = file_in_new_root.path();
const auto rest = [&] {
// chroot into new_root.
TEST_CHECK_SUCCESS(chroot(new_root.path().c_str()));
TEST_CHECK_SUCCESS(chroot(new_root_path.c_str()));
// getcwd should return "(unreachable)" followed by the initial_cwd.
char cwd[1024];
TEST_CHECK_SUCCESS(syscall(__NR_getcwd, cwd, sizeof(cwd)));
std::string expected_cwd = "(unreachable)";
expected_cwd += initial_cwd;
TEST_CHECK(strcmp(cwd, expected_cwd.c_str()) == 0);
char buf[1024];
TEST_CHECK_SUCCESS(syscall(__NR_getcwd, buf, sizeof(buf)));
constexpr char kUnreachablePrefix[] = "(unreachable)";
TEST_CHECK(
strncmp(buf, kUnreachablePrefix, sizeof(kUnreachablePrefix) - 1) == 0);
TEST_CHECK(strcmp(buf + sizeof(kUnreachablePrefix) - 1, initial_cwd) == 0);
// Should not be able to stat file by its full path.
struct stat statbuf;
TEST_CHECK_ERRNO(stat(file_in_new_root.path().c_str(), &statbuf), ENOENT);
TEST_CHECK_ERRNO(stat(file_in_new_root_path.c_str(), &statbuf), ENOENT);
// Should be able to stat file at new rooted path.
auto basename = std::string(Basename(file_in_new_root.path()));
auto rootedFile = "/" + basename;
TEST_CHECK_SUCCESS(stat(rootedFile.c_str(), &statbuf));
buf[0] = '/';
absl::string_view basename = Basename(file_in_new_root_path);
TEST_CHECK(basename.length() < (sizeof(buf) - 2));
memcpy(buf + 1, basename.data(), basename.length());
buf[basename.length() + 1] = '\0';
TEST_CHECK_SUCCESS(stat(buf, &statbuf));
// Should be able to stat cwd at '.' even though it's outside root.
TEST_CHECK_SUCCESS(stat(".", &statbuf));
@@ -131,8 +177,8 @@ TEST(ChrootTest, CreatesNewRoot) {
TEST_CHECK_SUCCESS(chdir("/"));
// getcwd should return "/".
TEST_CHECK_SUCCESS(syscall(__NR_getcwd, cwd, sizeof(cwd)));
TEST_CHECK_SUCCESS(strcmp(cwd, "/") == 0);
TEST_CHECK_SUCCESS(syscall(__NR_getcwd, buf, sizeof(buf)));
TEST_CHECK_SUCCESS(strcmp(buf, "/") == 0);
// Statting '.', '..', '/', and '/..' all return the same dev and inode.
struct stat statbuf_dot;
@@ -160,10 +206,11 @@ TEST(ChrootTest, DotDotFromOpenFD) {
auto fd = ASSERT_NO_ERRNO_AND_VALUE(
Open(dir_outside_root.path(), O_RDONLY | O_DIRECTORY));
auto new_root = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const std::string new_root_path = new_root.path();
const auto rest = [&] {
// chroot into new_root.
TEST_CHECK_SUCCESS(chroot(new_root.path().c_str()));
TEST_CHECK_SUCCESS(chroot(new_root_path.c_str()));
// openat on fd with path .. will succeed.
int other_fd;
@@ -184,15 +231,18 @@ TEST(ChrootTest, ProcFdLinkResolutionInChroot) {
const TempPath file_outside_chroot =
ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateFile());
const std::string file_outside_chroot_path = file_outside_chroot.path();
const FileDescriptor fd =
ASSERT_NO_ERRNO_AND_VALUE(Open(file_outside_chroot.path(), O_RDONLY));
const FileDescriptor proc_fd = ASSERT_NO_ERRNO_AND_VALUE(
Open("/proc", O_DIRECTORY | O_RDONLY | O_CLOEXEC));
auto temp_dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const std::string temp_dir_path = temp_dir.path();
const auto rest = [&] {
auto temp_dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
TEST_CHECK_SUCCESS(chroot(temp_dir.path().c_str()));
TEST_CHECK_SUCCESS(chroot(temp_dir_path.c_str()));
// Opening relative to an already open fd to a node outside the chroot
// works.
@@ -201,9 +251,10 @@ TEST(ChrootTest, ProcFdLinkResolutionInChroot) {
// Proc fd symlinks can escape the chroot if the fd the symlink refers to
// refers to an object outside the chroot.
char fd_buf[11];
TEST_CHECK(SafeItoa(fd.get(), fd_buf, sizeof(fd_buf), 10));
struct stat s = {};
TEST_CHECK_SUCCESS(
fstatat(proc_self_fd.get(), absl::StrCat(fd.get()).c_str(), &s, 0));
TEST_CHECK_SUCCESS(fstatat(proc_self_fd.get(), fd_buf, &s, 0));
// Try to stat the stdin fd. Internally, this is handled differently from a
// proc fd entry pointing to a file, since stdin is backed by a host fd, and
@@ -223,10 +274,12 @@ TEST(ChrootTest, ProcMemSelfFdsNoEscapeProcOpen) {
const FileDescriptor proc =
ASSERT_NO_ERRNO_AND_VALUE(Open("/proc", O_RDONLY));
const auto temp_dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const std::string temp_dir_path = temp_dir.path();
const auto rest = [&] {
// Create and enter a chroot directory.
const auto temp_dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
TEST_CHECK_SUCCESS(chroot(temp_dir.path().c_str()));
// Enter the chroot directory.
TEST_CHECK_SUCCESS(chroot(temp_dir_path.c_str()));
// Open a file inside the chroot at /foo.
const FileDescriptor foo =
@@ -234,11 +287,15 @@ TEST(ChrootTest, ProcMemSelfFdsNoEscapeProcOpen) {
// Examine /proc/self/fd/{foo_fd} to see if it exposes the fact that we're
// inside a chroot, the path should be /foo and NOT {chroot_dir}/foo.
const std::string fd_path = absl::StrCat("self/fd/", foo.get());
constexpr char kSelfFdRelpath[] = "self/fd/";
char path_buf[20];
strcpy(path_buf, kSelfFdRelpath); // NOLINT: need async-signal-safety
TEST_CHECK(SafeItoa(foo.get(), path_buf + sizeof(kSelfFdRelpath) - 1,
sizeof(path_buf) - (sizeof(kSelfFdRelpath) - 1), 10));
char buf[1024] = {};
size_t bytes_read = 0;
TEST_CHECK_SUCCESS(bytes_read = readlinkat(proc.get(), fd_path.c_str(), buf,
sizeof(buf) - 1));
TEST_CHECK_SUCCESS(
bytes_read = readlinkat(proc.get(), path_buf, buf, sizeof(buf) - 1));
// The link should resolve to something.
TEST_CHECK(bytes_read > 0);
@@ -258,10 +315,12 @@ TEST(ChrootTest, ProcMemSelfMapsNoEscapeProcOpen) {
const FileDescriptor proc =
ASSERT_NO_ERRNO_AND_VALUE(Open("/proc", O_RDONLY));
const auto temp_dir = TEST_CHECK_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const std::string temp_dir_path = temp_dir.path();
const auto rest = [&] {
// Create and enter a chroot directory.
const auto temp_dir = TEST_CHECK_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
TEST_CHECK_SUCCESS(chroot(temp_dir.path().c_str()));
// Enter the chroot directory.
TEST_CHECK_SUCCESS(chroot(temp_dir_path.c_str()));
// Open a file inside the chroot at /foo.
const FileDescriptor foo =
@@ -272,9 +331,12 @@ TEST(ChrootTest, ProcMemSelfMapsNoEscapeProcOpen) {
MAP_PRIVATE, foo.get(), 0);
TEST_CHECK_SUCCESS(reinterpret_cast<int64_t>(foo_map));
// Always unmap.
auto cleanup_map =
Cleanup([&] { TEST_CHECK_SUCCESS(munmap(foo_map, kPageSize)); });
// Always unmap. Since this function is called between fork() and execve(),
// we can't use gvisor::testing::Cleanup, which uses std::function
// and thus may heap-allocate (which is async-signal-unsafe); instead, use
// absl::Cleanup, which is templated on the callback type.
auto cleanup_map = absl::MakeCleanup(
[&] { TEST_CHECK_SUCCESS(munmap(foo_map, kPageSize)); });
// Examine /proc/self/maps to be sure that /foo doesn't appear to be
// mapped with the full chroot path.
@@ -289,8 +351,8 @@ TEST(ChrootTest, ProcMemSelfMapsNoEscapeProcOpen) {
TEST_CHECK(bytes_read > 0);
// Finally we want to make sure the maps don't contain the chroot path
TEST_CHECK(std::string(buf, bytes_read).find(temp_dir.path()) ==
std::string::npos);
TEST_CHECK(
!absl::StrContains(absl::string_view(buf, bytes_read), temp_dir_path));
};
EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0));
}
@@ -302,72 +364,72 @@ TEST(ChrootTest, ProcMountsMountinfoNoEscape) {
SKIP_IF(!ASSERT_NO_ERRNO_AND_VALUE(HaveCapability(CAP_SYS_CHROOT)));
// Create nested tmpfs mounts.
auto const outer_dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
auto const outer_mount = ASSERT_NO_ERRNO_AND_VALUE(
Mount("none", outer_dir.path(), "tmpfs", 0, "mode=0700", 0));
const auto outer_dir = ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDir());
const std::string outer_dir_path = outer_dir.path();
const auto outer_mount = ASSERT_NO_ERRNO_AND_VALUE(
Mount("none", outer_dir_path, "tmpfs", 0, "mode=0700", 0));
auto const inner_dir =
ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(outer_dir.path()));
auto const inner_mount = ASSERT_NO_ERRNO_AND_VALUE(
Mount("none", inner_dir.path(), "tmpfs", 0, "mode=0700", 0));
const auto inner_dir =
ASSERT_NO_ERRNO_AND_VALUE(TempPath::CreateDirIn(outer_dir_path));
const std::string inner_dir_path = inner_dir.path();
const auto inner_mount = ASSERT_NO_ERRNO_AND_VALUE(
Mount("none", inner_dir_path, "tmpfs", 0, "mode=0700", 0));
const std::string inner_dir_in_outer_chroot_path =
absl::StrCat("/", Basename(inner_dir_path));
const auto rest = [&outer_dir, &inner_dir] {
// Filenames that will be checked for mounts, all relative to /proc dir.
std::string paths[3] = {"mounts", "self/mounts", "self/mountinfo"};
// Filenames that will be checked for mounts, all relative to /proc dir.
std::string paths[3] = {"mounts", "self/mounts", "self/mountinfo"};
for (const std::string& path : paths) {
// We should have both inner and outer mounts.
const std::string contents =
TEST_CHECK_NO_ERRNO_AND_VALUE(GetContents(JoinPath("/proc", path)));
EXPECT_THAT(contents, AllOf(HasSubstr(outer_dir.path()),
HasSubstr(inner_dir.path())));
// We better have at least two mounts: the mounts we created plus the
// root.
std::vector<absl::string_view> submounts =
absl::StrSplit(contents, '\n', absl::SkipWhitespace());
TEST_CHECK(submounts.size() > 2);
}
for (const std::string& path : paths) {
// We should have both inner and outer mounts.
const std::string contents =
ASSERT_NO_ERRNO_AND_VALUE(GetContents(JoinPath("/proc", path)));
EXPECT_THAT(contents,
AllOf(HasSubstr(outer_dir_path), HasSubstr(inner_dir_path)));
// We better have at least two mounts: the mounts we created plus the
// root.
std::vector<absl::string_view> submounts =
absl::StrSplit(contents, '\n', absl::SkipWhitespace());
ASSERT_GT(submounts.size(), 2);
}
// Get a FD to /proc before we enter the chroot.
const FileDescriptor proc =
TEST_CHECK_NO_ERRNO_AND_VALUE(Open("/proc", O_RDONLY));
// Get a FD to /proc before we enter the chroot.
const FileDescriptor proc =
ASSERT_NO_ERRNO_AND_VALUE(Open("/proc", O_RDONLY));
const auto rest = [&] {
// Chroot to outer mount.
TEST_CHECK_SUCCESS(chroot(outer_dir.path().c_str()));
TEST_CHECK_SUCCESS(chroot(outer_dir_path.c_str()));
char buf[8 * 1024];
for (const std::string& path : paths) {
const FileDescriptor proc_file =
TEST_CHECK_NO_ERRNO_AND_VALUE(OpenAt(proc.get(), path, O_RDONLY));
// Only two mounts visible from this chroot: the inner and outer. Both
// paths should be relative to the new chroot.
const std::string contents =
TEST_CHECK_NO_ERRNO_AND_VALUE(GetContentsFD(proc_file.get()));
EXPECT_THAT(contents,
AllOf(HasSubstr(absl::StrCat(Basename(inner_dir.path()))),
Not(HasSubstr(outer_dir.path())),
Not(HasSubstr(inner_dir.path()))));
std::vector<absl::string_view> submounts =
absl::StrSplit(contents, '\n', absl::SkipWhitespace());
TEST_CHECK(submounts.size() == 2);
ssize_t n = ReadFd(proc_file.get(), buf, sizeof(buf));
TEST_PCHECK(n >= 0);
buf[n] = '\0';
TEST_CHECK(absl::StrContains(buf, Basename(inner_dir_path)));
TEST_CHECK(!absl::StrContains(buf, outer_dir_path));
TEST_CHECK(!absl::StrContains(buf, inner_dir_path));
TEST_CHECK(std::count(buf, buf + n, '\n') == 2);
}
// Chroot to inner mount. We must use an absolute path accessible to our
// chroot.
const std::string inner_dir_basename =
absl::StrCat("/", Basename(inner_dir.path()));
TEST_CHECK_SUCCESS(chroot(inner_dir_basename.c_str()));
TEST_CHECK_SUCCESS(chroot(inner_dir_in_outer_chroot_path.c_str()));
for (const std::string& path : paths) {
const FileDescriptor proc_file =
TEST_CHECK_NO_ERRNO_AND_VALUE(OpenAt(proc.get(), path, O_RDONLY));
const std::string contents =
TEST_CHECK_NO_ERRNO_AND_VALUE(GetContentsFD(proc_file.get()));
// Only the inner mount visible from this chroot.
std::vector<absl::string_view> submounts =
absl::StrSplit(contents, '\n', absl::SkipWhitespace());
TEST_CHECK(submounts.size() == 1);
ssize_t n = ReadFd(proc_file.get(), buf, sizeof(buf));
TEST_PCHECK(n >= 0);
buf[n] = '\0';
TEST_CHECK(std::count(buf, buf + n, '\n') == 1);
}
};
EXPECT_THAT(InForkedProcess(rest), IsPosixErrorOkAndHolds(0));