Do not go through magiskd for getting the log pipe

native/src/core/daemon.rs

@@ -5,7 +5,7 @@ use std::sync::{Mutex, OnceLock};
 use base::{cstr, Directory, ResultExt, Utf8CStr, Utf8CStrBuf, Utf8CStrBufRef, WalkResult};
 
 use crate::get_prop;
-use crate::logging::{magisk_logging, zygisk_logging};
+use crate::logging::magisk_logging;
 
 // Global magiskd singleton
 pub static MAGISKD: OnceLock<MagiskD> = OnceLock::new();
@@ -50,12 +50,6 @@ pub fn daemon_entry() {
     magisk_logging();
 }
 
-pub fn zygisk_entry() {
-    let magiskd = MagiskD::default();
-    MAGISKD.set(magiskd).ok();
-    zygisk_logging();
-}
-
 pub fn get_magiskd() -> &'static MagiskD {
     unsafe { MAGISKD.get().unwrap_unchecked() }
 }

native/src/core/lib.rs

@@ -3,7 +3,7 @@
 
 use base::Utf8CStr;
 use cert::read_certificate;
-use daemon::{daemon_entry, find_apk_path, get_magiskd, zygisk_entry, MagiskD};
+use daemon::{daemon_entry, find_apk_path, get_magiskd, MagiskD};
 use logging::{android_logging, magisk_logging, zygisk_logging};
 use resetprop::{persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop};
 
@@ -41,7 +41,6 @@ pub mod ffi {
     #[namespace = "rust"]
     extern "Rust" {
         fn daemon_entry();
-        fn zygisk_entry();
 
         type MagiskD;
         fn get_magiskd() -> &'static MagiskD;

native/src/core/logging.rs

@@ -3,8 +3,9 @@ use std::ffi::{c_char, c_void};
 use std::fmt::Write as FmtWrite;
 use std::fs::File;
 use std::io::{IoSlice, Read, Write};
-use std::os::fd::{AsRawFd, FromRawFd, IntoRawFd, RawFd};
+use std::os::fd::{AsRawFd, FromRawFd, RawFd};
 use std::ptr::null_mut;
+use std::sync::atomic::{AtomicI32, Ordering};
 use std::{fs, io};
 
 use bytemuck::{bytes_of, bytes_of_mut, write_zeroes, Pod, Zeroable};
@@ -42,9 +43,6 @@ type ThreadEntry = extern "C" fn(*mut c_void) -> *mut c_void;
 extern "C" {
     fn __android_log_write(prio: i32, tag: *const c_char, msg: *const c_char);
     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);
 }
 
@@ -113,13 +111,6 @@ struct LogMeta {
     tid: i32,
 }
 
-fn print_log_error(msg: &str, e: io::Error) {
-    let mut buf = Utf8CStrBufArr::default();
-    buf.write_fmt(format_args_nl!("Error in {}: {}", msg, e))
-        .ok();
-    write_android_log(level_to_prio(LogLevel::Error), &buf);
-}
-
 fn write_android_log(prio: i32, msg: &Utf8CStr) {
     unsafe {
         __android_log_write(prio, raw_cstr!("Magisk"), msg.as_ptr());
@@ -142,7 +133,14 @@ fn write_log_to_pipe(logd: &mut File, prio: i32, msg: &Utf8CStr) -> io::Result<u
 
     let io1 = IoSlice::new(bytes_of(&meta));
     let io2 = IoSlice::new(msg);
-    logd.write_vectored(&[io1, io2])
+    let result = logd.write_vectored(&[io1, io2]);
+    if let Err(e) = result.as_ref() {
+        let mut buf = Utf8CStrBufArr::default();
+        buf.write_fmt(format_args_nl!("Cannot write_log_to_pipe: {}", e))
+            .ok();
+        write_android_log(level_to_prio(LogLevel::Error), &buf);
+    }
+    result
 }
 
 fn magisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
@@ -160,20 +158,59 @@ fn magisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
     let result = write_log_to_pipe(logd, prio, msg);
 
     // If any error occurs, shut down the logd pipe
-    if let Err(e) = result {
-        print_log_error("magisk_log", e);
+    if result.is_err() {
         *guard = None;
     }
 }
 
+static ZYGISK_LOGD: AtomicI32 = AtomicI32::new(-1);
+
+#[no_mangle]
+extern "C" fn zygisk_close_logd() {
+    unsafe {
+        libc::close(ZYGISK_LOGD.swap(-1, Ordering::Relaxed));
+    }
+}
+
+#[no_mangle]
+extern "C" fn zygisk_get_logd() -> i32 {
+    ZYGISK_LOGD.load(Ordering::Relaxed)
+}
+
 fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
-    let mut logd = unsafe {
-        let fd = zygisk_fetch_logd();
+    // If we don't have the log pipe set, open the log pipe FIFO. This could actually happen
+    // multiple times in the zygote daemon (parent process) because we had to close this
+    // file descriptor to prevent crashing.
+    //
+    // For some reason, zygote sanitizes and checks FDs *before* forking. This results in the fact
+    // that *every* time before zygote forks, it has to close all logging related FDs in order
+    // to pass FD checks, just to have it re-initialized immediately after any
+    // logging happens ¯\_(ツ)_/¯.
+    //
+    // To be consistent with this behavior, we also have to close the log pipe to magiskd
+    // to make zygote NOT crash if necessary. We accomplish this by hooking __android_log_close
+    // and closing it at the same time as the rest of logging FDs.
+
+    let mut fd = ZYGISK_LOGD.load(Ordering::Relaxed);
     if fd < 0 {
+        android_logging();
+        let mut buf = Utf8CStrBufArr::default();
+        let path = FsPathBuf::new(&mut buf)
+            .join(get_magisk_tmp())
+            .join(LOG_PIPE!());
+        // Open as RW as sometimes it may block
+        fd = unsafe { libc::open(path.as_ptr(), O_RDWR | O_CLOEXEC) };
+        if fd >= 0 {
+            // Only re-enable zygisk logging if success
+            zygisk_logging();
+            unsafe {
+                libc::close(ZYGISK_LOGD.swap(fd, Ordering::Relaxed));
+            }
+        } else {
+            // Cannot talk to pipe, abort
             return;
         }
-        File::from_raw_fd(fd)
-    };
+    }
 
     // Block SIGPIPE
     let mut mask: sigset_t;
@@ -185,25 +222,25 @@ fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) {
         pthread_sigmask(SIG_BLOCK, &mask, &mut orig_mask);
     }
 
+    let result = {
+        let mut logd = unsafe { File::from_raw_fd(fd) };
         let result = write_log_to_pipe(&mut logd, prio, msg);
         // Make sure the file descriptor is not closed after out of scope
-    logd.into_raw_fd();
+        std::mem::forget(logd);
+        result
+    };
 
     // 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 let Err(e) = result {
-        print_log_error("zygisk_log", e);
-        unsafe {
+    if result.is_err() {
         zygisk_close_logd();
     }
-    }
 }
 
 // The following is implementation for the logging daemon

native/src/zygisk/entry.cpp

@@ -41,7 +41,7 @@ extern "C" void unload_first_stage() {
 }
 
 extern "C" void zygisk_inject_entry(void *handle) {
-    rust::zygisk_entry();
+    zygisk_logging();
     ZLOGD("load success\n");
 
     char *ld = getenv("LD_PRELOAD");
@@ -62,47 +62,6 @@ 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
-    // file descriptor to prevent crashing.
-    //
-    // For some reason, zygote sanitizes and checks FDs *before* forking. This results in the fact
-    // that *every* time before zygote forks, it has to close all logging related FDs in order
-    // to pass FD checks, just to have it re-initialized immediately after any
-    // logging happens ¯\_(ツ)_/¯.
-    //
-    // To be consistent with this behavior, we also have to close the log pipe to magiskd
-    // to make zygote NOT crash if necessary. For nativeForkAndSpecialize, we can actually
-    // 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 (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 zygisk_logd;
-}
-
-extern "C" void zygisk_close_logd() {
-    close(zygisk_logd);
-    zygisk_logd = -1;
-}
-
 static inline bool should_load_modules(uint32_t flags) {
     return (flags & UNMOUNT_MASK) != UNMOUNT_MASK &&
            (flags & PROCESS_IS_MAGISK_APP) != PROCESS_IS_MAGISK_APP;
@@ -332,16 +291,6 @@ static void get_process_info(int client, const sock_cred *cred) {
     }
 }
 
-static void send_log_pipe(int fd) {
-    int logd_fd = rust::get_magiskd().get_log_pipe();
-    if (logd_fd >= 0) {
-        write_int(fd, 0);
-        send_fd(fd, logd_fd);
-    } else {
-        write_int(fd, 1);
-    }
-}
-
 static void get_moddir(int client) {
     int id = read_int(client);
     char buf[4096];
@@ -364,9 +313,6 @@ void zygisk_handler(int client, const sock_cred *cred) {
     case ZygiskRequest::GET_INFO:
         get_process_info(client, cred);
         break;
-    case ZygiskRequest::GET_LOG_PIPE:
-        send_log_pipe(client);
-        break;
     case ZygiskRequest::CONNECT_COMPANION:
         if (get_exe(cred->pid, buf, sizeof(buf))) {
             connect_companion(client, str_ends(buf, "64"));

native/src/zygisk/hook.cpp

@@ -421,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 (zygisk_logd >= 0) {
-            allowed_fds[zygisk_logd] = false;
+        if (int fd = zygisk_get_logd(); fd >= 0) {
+            allowed_fds[fd] = false;
         }
     }
 
@@ -448,8 +448,8 @@ void HookContext::sanitize_fds() {
 
     auto &allowed_fds = *g_allowed_fds;
     if (can_exempt_fd()) {
-        if (zygisk_logd >= 0) {
-            exempted_fds.push_back(zygisk_logd);
+        if (int fd = zygisk_get_logd(); fd >= 0) {
+            exempted_fds.push_back(fd);
         }
 
         auto update_fd_array = [&](int old_len) -> jintArray {

native/src/zygisk/zygisk.hpp

@@ -15,7 +15,6 @@ namespace ZygiskRequest {
 enum : int {
     SETUP,
     GET_INFO,
-    GET_LOG_PIPE,
     CONNECT_COMPANION,
     GET_MODDIR,
     PASSTHROUGH,
@@ -37,7 +36,7 @@ enum : int {
 #define HIJACK_BIN HIJACK_BIN32
 #endif
 
-extern int zygisk_logd;
+extern "C" int zygisk_get_logd();
 extern "C" void zygisk_close_logd();
 
 // Unmap all pages matching the name