Merge changes from topic "bionic_fdsan"
* changes:
crasher: add close(fileno(FILE*)) and close(dirfd(DIR*)).
debuggerd_handler: use syscall(__NR_close) instead of close.
base: add support for tagged fd closure to unique_fd.
diff --git a/base/Android.bp b/base/Android.bp
index 3d80d97..46a0233 100644
--- a/base/Android.bp
+++ b/base/Android.bp
@@ -135,6 +135,7 @@
"strings_test.cpp",
"test_main.cpp",
"test_utils_test.cpp",
+ "unique_fd_test.cpp",
],
target: {
android: {
diff --git a/base/include/android-base/unique_fd.h b/base/include/android-base/unique_fd.h
index 6cfcd3f..019d337 100644
--- a/base/include/android-base/unique_fd.h
+++ b/base/include/android-base/unique_fd.h
@@ -42,10 +42,29 @@
//
// unique_fd is also known as ScopedFd/ScopedFD/scoped_fd; mentioned here to help
// you find this class if you're searching for one of those names.
+
+#if defined(__BIONIC__)
+#include <android/fdsan.h>
+#endif
+
namespace android {
namespace base {
struct DefaultCloser {
+#if defined(__BIONIC__)
+ static void Tag(int fd, void* old_addr, void* new_addr) {
+ uint64_t old_tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD,
+ reinterpret_cast<uint64_t>(old_addr));
+ uint64_t new_tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD,
+ reinterpret_cast<uint64_t>(new_addr));
+ android_fdsan_exchange_owner_tag(fd, old_tag, new_tag);
+ }
+ static void Close(int fd, void* addr) {
+ uint64_t tag = android_fdsan_create_owner_tag(ANDROID_FDSAN_OWNER_TYPE_UNIQUE_FD,
+ reinterpret_cast<uint64_t>(addr));
+ android_fdsan_close_with_tag(fd, tag);
+ }
+#else
static void Close(int fd) {
// Even if close(2) fails with EINTR, the fd will have been closed.
// Using TEMP_FAILURE_RETRY will either lead to EBADF or closing someone
@@ -53,40 +72,75 @@
// http://lkml.indiana.edu/hypermail/linux/kernel/0509.1/0877.html
::close(fd);
}
+#endif
};
template <typename Closer>
class unique_fd_impl final {
public:
- unique_fd_impl() : value_(-1) {}
+ unique_fd_impl() {}
- explicit unique_fd_impl(int value) : value_(value) {}
+ explicit unique_fd_impl(int fd) { reset(fd); }
~unique_fd_impl() { reset(); }
- unique_fd_impl(unique_fd_impl&& other) : value_(other.release()) {}
+ unique_fd_impl(unique_fd_impl&& other) { reset(other.release()); }
unique_fd_impl& operator=(unique_fd_impl&& s) {
- reset(s.release());
+ int fd = s.fd_;
+ s.fd_ = -1;
+ reset(fd, &s);
return *this;
}
- void reset(int new_value = -1) {
- if (value_ != -1) {
- Closer::Close(value_);
- }
- value_ = new_value;
- }
+ void reset(int new_value = -1) { reset(new_value, nullptr); }
- int get() const { return value_; }
+ int get() const { return fd_; }
operator int() const { return get(); }
int release() __attribute__((warn_unused_result)) {
- int ret = value_;
- value_ = -1;
+ tag(fd_, this, nullptr);
+ int ret = fd_;
+ fd_ = -1;
return ret;
}
private:
- int value_;
+ void reset(int new_value, void* previous_tag) {
+ if (fd_ != -1) {
+ close(fd_, this);
+ }
+
+ fd_ = new_value;
+ if (new_value != -1) {
+ tag(new_value, previous_tag, this);
+ }
+ }
+
+ int fd_ = -1;
+
+ // Template magic to use Closer::Tag if available, and do nothing if not.
+ // If Closer::Tag exists, this implementation is preferred, because int is a better match.
+ // If not, this implementation is SFINAEd away, and the no-op below is the only one that exists.
+ template <typename T = Closer>
+ static auto tag(int fd, void* old_tag, void* new_tag)
+ -> decltype(T::Tag(fd, old_tag, new_tag), void()) {
+ T::Tag(fd, old_tag, new_tag);
+ }
+
+ template <typename T = Closer>
+ static void tag(long, void*, void*) {
+ // No-op.
+ }
+
+ // Same as above, to select between Closer::Close(int) and Closer::Close(int, void*).
+ template <typename T = Closer>
+ static auto close(int fd, void* tag_value) -> decltype(T::Close(fd, tag_value), void()) {
+ T::Close(fd, tag_value);
+ }
+
+ template <typename T = Closer>
+ static auto close(int fd, void*) -> decltype(T::Close(fd), void()) {
+ T::Close(fd);
+ }
unique_fd_impl(const unique_fd_impl&);
void operator=(const unique_fd_impl&);
@@ -97,7 +151,8 @@
#if !defined(_WIN32)
// Inline functions, so that they can be used header-only.
-inline bool Pipe(unique_fd* read, unique_fd* write) {
+template <typename Closer>
+inline bool Pipe(unique_fd_impl<Closer>* read, unique_fd_impl<Closer>* write) {
int pipefd[2];
#if defined(__linux__)
@@ -121,7 +176,9 @@
return true;
}
-inline bool Socketpair(int domain, int type, int protocol, unique_fd* left, unique_fd* right) {
+template <typename Closer>
+inline bool Socketpair(int domain, int type, int protocol, unique_fd_impl<Closer>* left,
+ unique_fd_impl<Closer>* right) {
int sockfd[2];
if (socketpair(domain, type, protocol, sockfd) != 0) {
return false;
@@ -131,7 +188,8 @@
return true;
}
-inline bool Socketpair(int type, unique_fd* left, unique_fd* right) {
+template <typename Closer>
+inline bool Socketpair(int type, unique_fd_impl<Closer>* left, unique_fd_impl<Closer>* right) {
return Socketpair(AF_UNIX, type, 0, left, right);
}
diff --git a/base/unique_fd_test.cpp b/base/unique_fd_test.cpp
new file mode 100644
index 0000000..3fdf12a
--- /dev/null
+++ b/base/unique_fd_test.cpp
@@ -0,0 +1,54 @@
+/*
+ * Copyright (C) 2018 The Android Open Source Project
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "android-base/unique_fd.h"
+
+#include <gtest/gtest.h>
+
+#include <errno.h>
+#include <fcntl.h>
+#include <unistd.h>
+
+using android::base::unique_fd;
+
+TEST(unique_fd, unowned_close) {
+#if defined(__BIONIC__)
+ unique_fd fd(open("/dev/null", O_RDONLY));
+ EXPECT_DEATH(close(fd.get()), "incorrect tag");
+#endif
+}
+
+TEST(unique_fd, untag_on_release) {
+ unique_fd fd(open("/dev/null", O_RDONLY));
+ close(fd.release());
+}
+
+TEST(unique_fd, move) {
+ unique_fd fd(open("/dev/null", O_RDONLY));
+ unique_fd fd_moved = std::move(fd);
+ ASSERT_EQ(-1, fd.get());
+ ASSERT_GT(fd_moved.get(), -1);
+}
+
+TEST(unique_fd, unowned_close_after_move) {
+#if defined(__BIONIC__)
+ unique_fd fd(open("/dev/null", O_RDONLY));
+ unique_fd fd_moved = std::move(fd);
+ ASSERT_EQ(-1, fd.get());
+ ASSERT_GT(fd_moved.get(), -1);
+ EXPECT_DEATH(close(fd_moved.get()), "incorrect tag");
+#endif
+}
diff --git a/debuggerd/crasher/crasher.cpp b/debuggerd/crasher/crasher.cpp
index f31337d..f0fe1d0 100644
--- a/debuggerd/crasher/crasher.cpp
+++ b/debuggerd/crasher/crasher.cpp
@@ -183,6 +183,8 @@
fprintf(stderr, " exit call exit(1)\n");
fprintf(stderr, "\n");
fprintf(stderr, " fortify fail a _FORTIFY_SOURCE check\n");
+ fprintf(stderr, " fdsan_file close a file descriptor that's owned by a FILE*\n");
+ fprintf(stderr, " fdsan_dir close a file descriptor that's owned by a DIR*\n");
fprintf(stderr, " seccomp fail a seccomp check\n");
#if defined(__arm__)
fprintf(stderr, " kuser_helper_version call kuser_helper_version\n");
@@ -236,39 +238,45 @@
// Actions.
if (!strcasecmp(arg, "SIGSEGV-non-null")) {
- sigsegv_non_null();
+ sigsegv_non_null();
} else if (!strcasecmp(arg, "smash-stack")) {
- volatile int len = 128;
- return smash_stack(&len);
+ volatile int len = 128;
+ return smash_stack(&len);
} else if (!strcasecmp(arg, "stack-overflow")) {
- overflow_stack(nullptr);
+ overflow_stack(nullptr);
} else if (!strcasecmp(arg, "nostack")) {
- crashnostack();
+ crashnostack();
} else if (!strcasecmp(arg, "exit")) {
- exit(1);
+ exit(1);
} else if (!strcasecmp(arg, "call-null")) {
return crash_null();
} else if (!strcasecmp(arg, "crash") || !strcmp(arg, "SIGSEGV")) {
- return crash(42);
+ return crash(42);
} else if (!strcasecmp(arg, "abort")) {
- maybe_abort();
+ maybe_abort();
} else if (!strcasecmp(arg, "assert")) {
- __assert("some_file.c", 123, "false");
+ __assert("some_file.c", 123, "false");
} else if (!strcasecmp(arg, "assert2")) {
- __assert2("some_file.c", 123, "some_function", "false");
+ __assert2("some_file.c", 123, "some_function", "false");
} else if (!strcasecmp(arg, "fortify")) {
- char buf[10];
- __read_chk(-1, buf, 32, 10);
- while (true) pause();
+ char buf[10];
+ __read_chk(-1, buf, 32, 10);
+ while (true) pause();
+ } else if (!strcasecmp(arg, "fdsan_file")) {
+ FILE* f = fopen("/dev/null", "r");
+ close(fileno(f));
+ } else if (!strcasecmp(arg, "fdsan_dir")) {
+ DIR* d = opendir("/dev/");
+ close(dirfd(d));
} else if (!strcasecmp(arg, "LOG(FATAL)")) {
- LOG(FATAL) << "hello " << 123;
+ LOG(FATAL) << "hello " << 123;
} else if (!strcasecmp(arg, "LOG_ALWAYS_FATAL")) {
- LOG_ALWAYS_FATAL("hello %s", "world");
+ LOG_ALWAYS_FATAL("hello %s", "world");
} else if (!strcasecmp(arg, "LOG_ALWAYS_FATAL_IF")) {
- LOG_ALWAYS_FATAL_IF(true, "hello %s", "world");
+ LOG_ALWAYS_FATAL_IF(true, "hello %s", "world");
} else if (!strcasecmp(arg, "SIGFPE")) {
- raise(SIGFPE);
- return EXIT_SUCCESS;
+ raise(SIGFPE);
+ return EXIT_SUCCESS;
} else if (!strcasecmp(arg, "SIGILL")) {
#if defined(__aarch64__)
__asm__ volatile(".word 0\n");
@@ -280,28 +288,28 @@
#error
#endif
} else if (!strcasecmp(arg, "SIGTRAP")) {
- raise(SIGTRAP);
- return EXIT_SUCCESS;
+ raise(SIGTRAP);
+ return EXIT_SUCCESS;
} else if (!strcasecmp(arg, "fprintf-NULL")) {
- fprintf_null();
+ fprintf_null();
} else if (!strcasecmp(arg, "readdir-NULL")) {
- readdir_null();
+ readdir_null();
} else if (!strcasecmp(arg, "strlen-NULL")) {
- return strlen_null();
+ return strlen_null();
} else if (!strcasecmp(arg, "pthread_join-NULL")) {
- return pthread_join(0, nullptr);
+ return pthread_join(0, nullptr);
} else if (!strcasecmp(arg, "heap-usage")) {
- abuse_heap();
+ abuse_heap();
} else if (!strcasecmp(arg, "leak")) {
- leak();
+ leak();
} else if (!strcasecmp(arg, "SIGSEGV-unmapped")) {
- char* map = reinterpret_cast<char*>(mmap(nullptr, sizeof(int), PROT_READ | PROT_WRITE,
- MAP_SHARED | MAP_ANONYMOUS, -1, 0));
- munmap(map, sizeof(int));
- map[0] = '8';
+ char* map = reinterpret_cast<char*>(
+ mmap(nullptr, sizeof(int), PROT_READ | PROT_WRITE, MAP_SHARED | MAP_ANONYMOUS, -1, 0));
+ munmap(map, sizeof(int));
+ map[0] = '8';
} else if (!strcasecmp(arg, "seccomp")) {
- set_system_seccomp_filter();
- syscall(99999);
+ set_system_seccomp_filter();
+ syscall(99999);
#if defined(__arm__)
} else if (!strcasecmp(arg, "kuser_helper_version")) {
return __kuser_helper_version;
diff --git a/debuggerd/handler/debuggerd_handler.cpp b/debuggerd/handler/debuggerd_handler.cpp
index c07a34a..615fb46 100644
--- a/debuggerd/handler/debuggerd_handler.cpp
+++ b/debuggerd/handler/debuggerd_handler.cpp
@@ -59,7 +59,16 @@
#include "protocol.h"
using android::base::Pipe;
-using android::base::unique_fd;
+
+// We muck with our fds in a 'thread' that doesn't share the same fd table.
+// Close fds in that thread with a raw close syscall instead of going through libc.
+struct FdsanBypassCloser {
+ static void Close(int fd) {
+ syscall(__NR_close, fd);
+ }
+};
+
+using unique_fd = android::base::unique_fd_impl<FdsanBypassCloser>;
// see man(2) prctl, specifically the section about PR_GET_NAME
#define MAX_TASK_NAME_LEN (16)
@@ -299,7 +308,8 @@
debugger_thread_info* thread_info = static_cast<debugger_thread_info*>(arg);
for (int i = 0; i < 1024; ++i) {
- close(i);
+ // Don't use close to avoid bionic's file descriptor ownership checks.
+ syscall(__NR_close, i);
}
int devnull = TEMP_FAILURE_RETRY(open("/dev/null", O_RDWR));