From 67cc36268e5b63136536ed16b0f3e4864efd76a4 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Thu, 26 Oct 2023 18:13:56 -0700 Subject: [PATCH] Simplify zygisk log pipe --- native/src/core/logging.rs | 31 +++++++++++-------------------- native/src/zygisk/entry.cpp | 30 +++++++++++++++++++++--------- native/src/zygisk/hook.cpp | 21 +++++++++------------ native/src/zygisk/zygisk.hpp | 3 +++ 4 files changed, 44 insertions(+), 41 deletions(-) diff --git a/native/src/core/logging.rs b/native/src/core/logging.rs index bac162033..97f325551 100644 --- a/native/src/core/logging.rs +++ b/native/src/core/logging.rs @@ -44,6 +44,7 @@ extern "C" { fn strftime(buf: *mut c_char, len: usize, fmt: *const c_char, tm: *const tm) -> usize; fn zygisk_fetch_logd() -> RawFd; + fn zygisk_close_logd(); fn new_daemon_thread(entry: ThreadEntry, arg: *mut c_void); } @@ -159,26 +160,13 @@ fn magisk_log_to_pipe(prio: i32, msg: &Utf8CStr) { } fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) { - let magiskd = match MAGISKD.get() { - None => return, - Some(s) => s, - }; - - let logd_cell = magiskd.logd.lock().unwrap(); - let mut logd_ref = logd_cell.borrow_mut(); - if logd_ref.is_none() { - android_logging(); - unsafe { - let fd = zygisk_fetch_logd(); - if fd < 0 { - return; - } - *logd_ref = Some(File::from_raw_fd(fd)); + let mut logd = unsafe { + let fd = zygisk_fetch_logd(); + if fd < 0 { + return; } - // Only re-enable zygisk logging if success - zygisk_logging(); + File::from_raw_fd(fd) }; - let logd = logd_ref.as_mut().unwrap(); // Block SIGPIPE let mut mask: sigset_t; @@ -190,18 +178,21 @@ fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) { pthread_sigmask(SIG_BLOCK, &mask, &mut orig_mask); } - let result = write_log_to_pipe(logd, prio, msg); + let result = write_log_to_pipe(&mut logd, prio, msg); // Consume SIGPIPE if exists, then restore mask unsafe { let ts: timespec = std::mem::zeroed(); + sigtimedwait(&mask, null_mut(), &ts); pthread_sigmask(SIG_SETMASK, &orig_mask, null_mut()); } // If any error occurs, shut down the logd pipe if result.is_err() { - *logd_ref = None; + unsafe { + zygisk_close_logd(); + } } } diff --git a/native/src/zygisk/entry.cpp b/native/src/zygisk/entry.cpp index becbf1166..c021ead11 100644 --- a/native/src/zygisk/entry.cpp +++ b/native/src/zygisk/entry.cpp @@ -62,6 +62,8 @@ extern "C" void zygisk_inject_entry(void *handle) { // The following code runs in zygote/app process +int zygisk_logd = -1; + extern "C" int zygisk_fetch_logd() { // If we don't have the log pipe set, request magiskd for it. This could actually happen // multiple times in the zygote daemon (parent process) because we had to close this @@ -77,18 +79,28 @@ extern "C" int zygisk_fetch_logd() { // add this FD into fds_to_ignore to pass the check. For other cases, we accomplish this by // hooking __android_log_close and closing it at the same time as the rest of logging FDs. - if (int fd = zygisk_request(ZygiskRequest::GET_LOG_PIPE); fd >= 0) { - int log_pipe = -1; - if (read_int(fd) == 0) { - log_pipe = recv_fd(fd); - } - close(fd); - if (log_pipe >= 0) { - return log_pipe; + if (zygisk_logd < 0) { + android_logging(); + if (int fd = zygisk_request(ZygiskRequest::GET_LOG_PIPE); fd >= 0) { + int log_pipe = -1; + if (read_int(fd) == 0) { + log_pipe = recv_fd(fd); + } + close(fd); + if (log_pipe >= 0) { + zygisk_logd = log_pipe; + // Only re-enable zygisk logging if success + zygisk_logging(); + } } } - return -1; + return zygisk_logd; +} + +extern "C" void zygisk_close_logd() { + close(zygisk_logd); + zygisk_logd = -1; } static inline bool should_load_modules(uint32_t flags) { diff --git a/native/src/zygisk/hook.cpp b/native/src/zygisk/hook.cpp index 0587c9d3a..8b6ecacfa 100644 --- a/native/src/zygisk/hook.cpp +++ b/native/src/zygisk/hook.cpp @@ -20,8 +20,6 @@ using namespace std; using jni_hook::hash_map; using jni_hook::tree_map; using xstring = jni_hook::string; -using rust::MagiskD; -using rust::get_magiskd; // Extreme verbose logging //#define ZLOGV(...) ZLOGD(__VA_ARGS__) @@ -70,7 +68,6 @@ struct HookContext { AppSpecializeArgs_v3 *app; ServerSpecializeArgs_v1 *server; } args; - const MagiskD &magiskd; const char *process; list modules; @@ -97,7 +94,7 @@ struct HookContext { vector ignore_info; HookContext(JNIEnv *env, void *args) : - env(env), args{args}, magiskd(get_magiskd()), process(nullptr), pid(-1), info_flags(0), + env(env), args{args}, process(nullptr), pid(-1), info_flags(0), hook_info_lock(PTHREAD_MUTEX_INITIALIZER) { static bool restored_env = false; if (!restored_env) { @@ -174,7 +171,7 @@ DCL_HOOK_FUNC(int, unshare, int flags) { DCL_HOOK_FUNC(void, android_log_close) { if (g_ctx == nullptr) { // Happens during un-managed fork like nativeForkApp, nativeForkUsap - get_magiskd().close_log_pipe(); + zygisk_close_logd(); } else { g_ctx->sanitize_fds(); } @@ -424,8 +421,8 @@ void HookContext::fork_pre() { // 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; + if (zygisk_logd >= 0) { + allowed_fds[zygisk_logd] = false; } } @@ -445,14 +442,14 @@ void HookContext::sanitize_fds() { return; if (!is_child() || g_allowed_fds == nullptr) { - magiskd.close_log_pipe(); + zygisk_close_logd(); 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); + if (zygisk_logd >= 0) { + exempted_fds.push_back(zygisk_logd); } auto update_fd_array = [&](int old_len) -> jintArray { @@ -492,7 +489,7 @@ void HookContext::sanitize_fds() { update_fd_array(0); } } else { - magiskd.close_log_pipe(); + zygisk_close_logd(); // Switch to plain old android logging because we cannot talk // to magiskd to fetch our log pipe afterwards anyways. android_logging(); @@ -589,7 +586,7 @@ void HookContext::app_specialize_post() { // Cleanups env->ReleaseStringUTFChars(args.app->nice_name, process); - magiskd.close_log_pipe(); + zygisk_close_logd(); android_logging(); } diff --git a/native/src/zygisk/zygisk.hpp b/native/src/zygisk/zygisk.hpp index 58ebffb12..8acee474f 100644 --- a/native/src/zygisk/zygisk.hpp +++ b/native/src/zygisk/zygisk.hpp @@ -37,6 +37,9 @@ enum : int { #define HIJACK_BIN HIJACK_BIN32 #endif +extern int zygisk_logd; +extern "C" void zygisk_close_logd(); + // Unmap all pages matching the name void unmap_all(const char *name);