Skip to content

Commit

Permalink
Simplify fd sanitization
Browse files Browse the repository at this point in the history
  • Loading branch information
@topjohnwu
topjohnwu committed Sep 29, 2023
1 parent 5c92d39 commit 79a1c39
Showing 1 changed file with 51 additions and 50 deletions.
101 changes: 51 additions & 50 deletions native/src/zygisk/hook.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ enum {
FLAG_MAX
};

#define MAX_FD_SIZE 1024

// Global variables
vector<tuple<dev_t, ino_t, const char *, void **>> *plt_hook_list;
map<string, vector<JNINativeMethod>, StringCmp> *jni_hook_list;
Expand All @@ -53,15 +55,14 @@ bool should_unmap_zygisk = false;

// Current context
HookContext *g_ctx;
bitset<MAX_FD_SIZE> *g_allowed_fds = nullptr;
const JNINativeInterface *old_functions = nullptr;
JNINativeInterface *new_functions = nullptr;

#define DCL_PRE_POST(name) \
void name##_pre(); \
void name##_post();

#define MAX_FD_SIZE 1024

struct HookContext {
JNIEnv *env;
union {
Expand All @@ -77,7 +78,6 @@ struct HookContext {
int pid;
bitset<FLAG_MAX> flags;
uint32_t info_flags;
bitset<MAX_FD_SIZE> allowed_fds;
vector<int> exempted_fds;

struct RegisterInfo {
Expand Down Expand Up @@ -121,6 +121,7 @@ struct HookContext {
void sanitize_fds();
bool exempt_fd(int fd);
bool is_child() const { return pid <= 0; }
bool can_exempt_fd() const { return flags[APP_FORK_AND_SPECIALIZE] && args.app->fds_to_ignore; }

// Compatibility shim
void plt_hook_register(const char *regex, const char *symbol, void *fn, void **backup);
Expand Down Expand Up @@ -169,19 +170,13 @@ DCL_HOOK_FUNC(int, unshare, int flags) {
return res;
}

// Close logd_fd if necessary to prevent crashing
// For more info, check comments in zygisk_log_write
// Sanitize file descriptors to prevent crashing
DCL_HOOK_FUNC(void, android_log_close) {
if (g_ctx == nullptr) {
// Happens during un-managed fork like nativeForkApp, nativeForkUsap
get_magiskd().close_log_pipe();
} else if (!g_ctx->flags[SKIP_FD_SANITIZATION]) {
g_ctx->magiskd.close_log_pipe();
if (g_ctx->is_child()) {
// Switch to plain old android logging because we cannot talk
// to magiskd to fetch our log pipe afterwards anyways.
android_logging();
}
} else {
g_ctx->sanitize_fds();
}
old_android_log_close();
}
Expand Down Expand Up @@ -412,26 +407,32 @@ int sigmask(int how, int signum) {
}

void HookContext::fork_pre() {
if (g_allowed_fds == nullptr) {
default_new(g_allowed_fds);

auto &allowed_fds = *g_allowed_fds;
// Record all open fds
auto dir = xopen_dir("/proc/self/fd");
for (dirent *entry; (entry = xreaddir(dir.get()));) {
int fd = parse_int(entry->d_name);
if (fd < 0 || fd >= MAX_FD_SIZE) {
close(fd);
continue;
}
allowed_fds[fd] = true;
}
// The dirfd will be closed once out of scope
allowed_fds[dirfd(dir.get())] = false;
// logd_fd should be handled separately
if (int logd_fd = magiskd.get_log_pipe(); logd_fd >= 0) {
allowed_fds[logd_fd] = false;
}
}

// Do our own fork before loading any 3rd party code
// First block SIGCHLD, unblock after original fork is done
sigmask(SIG_BLOCK, SIGCHLD);
pid = old_fork();

if (pid != 0 || flags[SKIP_FD_SANITIZATION])
return;

// Record all open fds
auto dir = xopen_dir("/proc/self/fd");
for (dirent *entry; (entry = xreaddir(dir.get()));) {
int fd = parse_int(entry->d_name);
if (fd < 0 || fd >= MAX_FD_SIZE) {
close(fd);
continue;
}
allowed_fds[fd] = true;
}
// The dirfd should not be allowed
allowed_fds[dirfd(dir.get())] = false;
}

void HookContext::fork_post() {
Expand All @@ -443,17 +444,27 @@ void HookContext::sanitize_fds() {
if (flags[SKIP_FD_SANITIZATION])
return;

if (flags[APP_FORK_AND_SPECIALIZE] && args.app->fds_to_ignore) {
auto update_fd_array = [&](int off) -> jintArray {
if (!is_child() || g_allowed_fds == nullptr) {
magiskd.close_log_pipe();
return;
}

auto &allowed_fds = *g_allowed_fds;
if (can_exempt_fd()) {
if (int logd_fd = magiskd.get_log_pipe(); logd_fd >= 0) {
exempted_fds.push_back(logd_fd);
}

auto update_fd_array = [&](int old_len) -> jintArray {
if (exempted_fds.empty())
return nullptr;

jintArray array = env->NewIntArray(static_cast<int>(off + exempted_fds.size()));
jintArray array = env->NewIntArray(static_cast<int>(old_len + exempted_fds.size()));
if (array == nullptr)
return nullptr;

env->SetIntArrayRegion(array, off, static_cast<int>(exempted_fds.size()),
exempted_fds.data());
env->SetIntArrayRegion(
array, old_len, static_cast<int>(exempted_fds.size()), exempted_fds.data());
for (int fd : exempted_fds) {
if (fd >= 0 && fd < MAX_FD_SIZE) {
allowed_fds[fd] = true;
Expand All @@ -480,15 +491,19 @@ void HookContext::sanitize_fds() {
} else {
update_fd_array(0);
}
} else {
magiskd.close_log_pipe();
// Switch to plain old android logging because we cannot talk
// to magiskd to fetch our log pipe afterwards anyways.
android_logging();
}

// Close all forbidden fds to prevent crashing
auto dir = xopen_dir("/proc/self/fd");
int dfd = dirfd(dir.get());
for (dirent *entry; (entry = xreaddir(dir.get()));) {
int fd = parse_int(entry->d_name);
int logd_fd = magiskd.get_log_pipe();
if ((fd < 0 || fd >= MAX_FD_SIZE || !allowed_fds[fd]) && fd != dfd && fd != logd_fd) {
if ((fd < 0 || fd >= MAX_FD_SIZE || !allowed_fds[fd]) && fd != dfd) {
close(fd);
}
}
Expand Down Expand Up @@ -645,7 +660,7 @@ HookContext::~HookContext() {
bool HookContext::exempt_fd(int fd) {
if (flags[POST_SPECIALIZE] || flags[SKIP_FD_SANITIZATION])
return true;
if (!flags[APP_FORK_AND_SPECIALIZE])
if (!can_exempt_fd())
return false;
exempted_fds.push_back(fd);
return true;
Expand Down Expand Up @@ -673,7 +688,6 @@ void HookContext::nativeForkSystemServer_pre() {
fork_pre();
if (is_child()) {
server_specialize_pre();
sanitize_fds();
}
}

Expand All @@ -688,24 +702,11 @@ void HookContext::nativeForkSystemServer_post() {
void HookContext::nativeForkAndSpecialize_pre() {
process = env->GetStringUTFChars(args.app->nice_name, nullptr);
ZLOGV("pre forkAndSpecialize [%s]\n", process);

flags[APP_FORK_AND_SPECIALIZE] = true;
if (args.app->fds_to_ignore == nullptr) {
// if fds_to_ignore does not exist, we do not have a good way to determine
// whether keeping fd open during fork is allowed, as needed symbols may be
// inlined. Better be safe than sorry.
flags[SKIP_FD_SANITIZATION] = false;
} else {
int logd_fd = magiskd.get_log_pipe();
if (logd_fd >= 0) {
exempted_fds.push_back(logd_fd);
}
}

fork_pre();
if (is_child()) {
app_specialize_pre();
sanitize_fds();
}
}

Expand Down

0 comments on commit 79a1c39

Please sign in to comment.