tequilaOS/platform_bionic
2
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

Disable fdtrack post-fork.

Browse source 
Also delete some fdsan code that attempts to check for the post-fork
state, but never will, because we update the cached pid upon fork.

Bug: http://b/174542867
Test: /data/nativetest64/bionic-unit-tests/bionic-unit-tests
Test: treehugger
Change-Id: I9b748dac9de9b4c741897d93e64d31737e52bf8e
This commit is contained in:
Josh Gao 2020-12-09 14:01:13 -08:00
parent 87a205eefa
commit dcc97c0887
8 changed files with 80 additions and 50 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

8
libc/bionic/fdsan.cpp
View file

@ -137,14 +137,6 @@ __printflike(1, 0) static void fdsan_error(const char* fmt, ...) {
return;
}
// Lots of code will (sensibly) fork, call close on all of their fds,
// and then exec. Compare our cached pid value against the real one to detect
// this scenario and permit it.
pid_t cached_pid = __get_cached_pid();
if (cached_pid == 0 || cached_pid != syscall(__NR_getpid)) {
return;
}
struct {
size_t size;
char buf[512];

8
libc/bionic/fdtrack.cpp
View file

@ -37,8 +37,14 @@
_Atomic(android_fdtrack_hook_t) __android_fdtrack_hook;
bool __android_fdtrack_globally_disabled = false;
void android_fdtrack_set_globally_enabled(bool new_value) {
__android_fdtrack_globally_disabled = !new_value;
}
bool android_fdtrack_get_enabled() {
return !__get_bionic_tls().fdtrack_disabled;
return !__get_bionic_tls().fdtrack_disabled && !__android_fdtrack_globally_disabled;
}
bool android_fdtrack_set_enabled(bool new_value) {

4
libc/bionic/fork.cpp
View file

@ -31,6 +31,7 @@
#include <android/fdsan.h>
#include "private/bionic_defs.h"
#include "private/bionic_fdtrack.h"
#include "pthread_internal.h"
__BIONIC_WEAK_FOR_NATIVE_BRIDGE_INLINE
@ -55,9 +56,10 @@ int fork() {
int result = __clone_for_fork();
if (result == 0) {
// Disable fdsan post-fork, so we don't falsely trigger on processes that
// Disable fdsan and fdtrack post-fork, so we don't falsely trigger on processes that
// fork, close all of their fds, and then exec.
android_fdsan_set_error_level(ANDROID_FDSAN_ERROR_LEVEL_DISABLED);
android_fdtrack_set_globally_enabled(false);
// Reset the stack_and_tls VMA name so it doesn't end with a tid from the
// parent process.

1
libc/libc.map.txt
View file

@ -1772,6 +1772,7 @@ LIBC_PLATFORM {
android_fdtrack_compare_exchange_hook; # llndk
android_fdtrack_get_enabled; # llndk
android_fdtrack_set_enabled; # llndk
android_fdtrack_set_globally_enabled; # llndk
android_net_res_stats_get_info_for_net;
android_net_res_stats_aggregate;
android_net_res_stats_get_usable_servers;

4
libc/platform/bionic/fdtrack.h
View file

@ -70,4 +70,8 @@ bool android_fdtrack_compare_exchange_hook(android_fdtrack_hook_t* expected, and
bool android_fdtrack_get_enabled() __INTRODUCED_IN(30);
bool android_fdtrack_set_enabled(bool new_value) __INTRODUCED_IN(30);
// Globally enable/disable fdtrack.
// This is primaryily useful to reenable fdtrack after it's been automatically disabled post-fork.
void android_fdtrack_set_globally_enabled(bool new_value) __INTRODUCED_IN(31);
__END_DECLS

15
libc/private/bionic_fdtrack.h
View file

@ -28,16 +28,17 @@
#pragma once
#include <sys/cdefs.h>
#include <stdatomic.h>
#include <sys/cdefs.h>
#include "platform/bionic/fdtrack.h"
#include "bionic/pthread_internal.h"
#include "private/bionic_tls.h"
#include "private/ErrnoRestorer.h"
#include "private/bionic_tls.h"
extern "C" _Atomic(android_fdtrack_hook_t) __android_fdtrack_hook;
extern "C" bool __android_fdtrack_globally_disabled;
// Macro to record file descriptor creation.
// e.g.:
@ -51,14 +52,15 @@ extern "C" _Atomic(android_fdtrack_hook_t) __android_fdtrack_hook;
!__predict_false(__get_thread()->is_vforked())) { \
bionic_tls& tls = __get_bionic_tls(); \
/* fdtrack_disabled is only true during reentrant calls. */ \
if (!__predict_false(tls.fdtrack_disabled)) { \
if (!__predict_false(tls.fdtrack_disabled) && \
!__predict_false(__android_fdtrack_globally_disabled)) { \
ErrnoRestorer r; \
tls.fdtrack_disabled = true; \
android_fdtrack_event event; \
event.fd = __fd; \
event.type = ANDROID_FDTRACK_EVENT_TYPE_CREATE; \
event.data.create.function_name = name; \
atomic_load(&__android_fdtrack_hook)(&event); \
atomic_load (&__android_fdtrack_hook)(&event); \
tls.fdtrack_disabled = false; \
} \
} \
@ -80,13 +82,14 @@ extern "C" _Atomic(android_fdtrack_hook_t) __android_fdtrack_hook;
if (__fd != -1 && __predict_false(__android_fdtrack_hook) && \
!__predict_false(__get_thread()->is_vforked())) { \
bionic_tls& tls = __get_bionic_tls(); \
if (!__predict_false(tls.fdtrack_disabled)) { \
if (!__predict_false(tls.fdtrack_disabled) && \
!__predict_false(__android_fdtrack_globally_disabled)) { \
int saved_errno = errno; \
tls.fdtrack_disabled = true; \
android_fdtrack_event event; \
event.fd = __fd; \
event.type = ANDROID_FDTRACK_EVENT_TYPE_CLOSE; \
atomic_load(&__android_fdtrack_hook)(&event); \
atomic_load (&__android_fdtrack_hook)(&event); \
tls.fdtrack_disabled = false; \
errno = saved_errno; \
} \

2
libfdtrack/fdtrack.cpp
View file

@ -93,6 +93,8 @@ __attribute__((constructor)) static void ctor() {
android_fdtrack_hook_t expected = nullptr;
installed = android_fdtrack_compare_exchange_hook(&expected, &fd_hook);
}
android_fdtrack_set_globally_enabled(true);
}
__attribute__((destructor)) static void dtor() {

22
tests/fdtrack_test.cpp
View file

@ -57,8 +57,13 @@ void DumpEvent(std::vector<android_fdtrack_event>* events, size_t index) {
}
}
std::vector<android_fdtrack_event> FdtrackRun(void (*func)()) {
std::vector<android_fdtrack_event> FdtrackRun(void (*func)(), bool reenable = true) {
// Each bionic test is run in separate process, so we can safely use a static here.
// However, since they're all forked, we need to reenable fdtrack.
if (reenable) {
android_fdtrack_set_globally_enabled(true);
}
static std::vector<android_fdtrack_event> events;
events.clear();
@ -129,6 +134,21 @@ TEST(fdtrack, close) {
#endif
}
TEST(fdtrack, fork) {
#if defined(__BIONIC__)
ASSERT_EXIT(
[]() {
static int fd = open("/dev/null", O_WRONLY | O_CLOEXEC);
ASSERT_NE(-1, fd);
auto events = FdtrackRun([]() { close(fd); }, false);
ASSERT_EQ(0U, events.size());
exit(0);
}(),
testing::ExitedWithCode(0), "");
#endif
}
TEST(fdtrack, enable_disable) {
#if defined(__BIONIC__)
static int fd1 = -1;