From 0a2a590ab72d922a9b761b7e2d6a9d97072a0c2f Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Tue, 12 Sep 2023 17:35:20 -0700 Subject: [PATCH] Use Utf8CStr for logging --- native/src/base/logging.cpp | 2 +- native/src/base/logging.rs | 14 ++++++++------ native/src/core/logging.rs | 28 ++++++++++++++-------------- native/src/init/logging.rs | 17 +++++++---------- 4 files changed, 30 insertions(+), 31 deletions(-) diff --git a/native/src/base/logging.cpp b/native/src/base/logging.cpp index 41b165442..8157c1e7c 100644 --- a/native/src/base/logging.cpp +++ b/native/src/base/logging.cpp @@ -15,7 +15,7 @@ static int fmt_and_log_with_rs(LogLevel level, const char *fmt, va_list ap) { buf[0] = '\0'; // Fortify logs when a fatal error occurs. Do not run through fortify again int len = std::min(__call_bypassing_fortify(vsnprintf)(buf, sz, fmt, ap), sz - 1); - log_with_rs(level, byte_view(buf, len)); + log_with_rs(level, byte_view(buf, len + 1)); return len; } diff --git a/native/src/base/logging.rs b/native/src/base/logging.rs index a87cb5d48..28794cb6d 100644 --- a/native/src/base/logging.rs +++ b/native/src/base/logging.rs @@ -5,7 +5,7 @@ use std::panic::Location; use std::process::exit; use crate::ffi::LogLevel; -use crate::Utf8CStrArr; +use crate::{Utf8CStr, Utf8CStrArr}; // Error handling and logging throughout the Rust codebase in Magisk: // @@ -37,7 +37,7 @@ pub static mut LOGGER: Logger = Logger { flags: 0, }; -type LogWriter = fn(level: LogLevel, msg: &[u8]); +type LogWriter = fn(level: LogLevel, msg: &Utf8CStr); type Formatter<'a> = &'a mut dyn fmt::Write; #[derive(Copy, Clone)] @@ -91,6 +91,8 @@ fn log_with_writer(level: LogLevel, f: F) { } pub fn log_from_cxx(level: LogLevel, msg: &[u8]) { + // SAFETY: The null termination is handled on the C++ side + let msg = unsafe { Utf8CStr::from_bytes_unchecked(msg) }; log_with_writer(level, |write| write(level, msg)); } @@ -98,7 +100,7 @@ pub fn log_with_formatter fmt::Result>(level: LogLevel, log_with_writer(level, |write| { let mut buf = Utf8CStrArr::default(); f(&mut buf).ok(); - write(level, buf.as_bytes_with_nul()); + write(level, &buf); }); } @@ -107,11 +109,11 @@ pub fn log_with_args(level: LogLevel, args: Arguments) { } pub fn cmdline_logging() { - fn cmdline_write(level: LogLevel, msg: &[u8]) { + fn cmdline_write(level: LogLevel, msg: &Utf8CStr) { if level == LogLevel::Info { - stdout().write_all(msg).ok(); + stdout().write_all(msg.as_bytes()).ok(); } else { - stderr().write_all(msg).ok(); + stderr().write_all(msg.as_bytes()).ok(); } } diff --git a/native/src/core/logging.rs b/native/src/core/logging.rs index 31801a878..045b642f8 100644 --- a/native/src/core/logging.rs +++ b/native/src/core/logging.rs @@ -40,7 +40,7 @@ enum ALogPriority { 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 u8); + 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; @@ -58,7 +58,7 @@ fn level_to_prio(level: LogLevel) -> i32 { } pub fn android_logging() { - fn android_log_write(level: LogLevel, msg: &[u8]) { + fn android_log_write(level: LogLevel, msg: &Utf8CStr) { unsafe { __android_log_write(level_to_prio(level), raw_cstr!("Magisk"), msg.as_ptr()); } @@ -75,15 +75,15 @@ pub fn android_logging() { } pub fn magisk_logging() { - fn magisk_write(level: LogLevel, msg: &[u8]) { + fn magisk_log_write(level: LogLevel, msg: &Utf8CStr) { unsafe { __android_log_write(level_to_prio(level), raw_cstr!("Magisk"), msg.as_ptr()); } - magisk_log_write(level_to_prio(level), msg); + magisk_log_to_pipe(level_to_prio(level), msg); } let logger = Logger { - write: magisk_write, + write: magisk_log_write, flags: 0, }; exit_on_error(false); @@ -93,15 +93,15 @@ pub fn magisk_logging() { } pub fn zygisk_logging() { - fn zygisk_write(level: LogLevel, msg: &[u8]) { + fn zygisk_log_write(level: LogLevel, msg: &Utf8CStr) { unsafe { __android_log_write(level_to_prio(level), raw_cstr!("Magisk"), msg.as_ptr()); } - zygisk_log_write(level_to_prio(level), msg); + zygisk_log_to_pipe(level_to_prio(level), msg); } let logger = Logger { - write: zygisk_write, + write: zygisk_log_write, flags: 0, }; exit_on_error(false); @@ -121,10 +121,10 @@ struct LogMeta { const MAX_MSG_LEN: usize = PIPE_BUF - std::mem::size_of::(); -fn do_magisk_log_write(logd: &mut File, prio: i32, msg: &[u8]) -> io::Result { +fn write_log_to_pipe(logd: &mut File, prio: i32, msg: &Utf8CStr) -> io::Result { // Truncate message if needed let len = min(MAX_MSG_LEN, msg.len()); - let msg = &msg[..len]; + let msg = &msg.as_bytes()[..len]; let meta = LogMeta { prio, @@ -138,7 +138,7 @@ fn do_magisk_log_write(logd: &mut File, prio: i32, msg: &[u8]) -> io::Result return, Some(s) => s, @@ -151,7 +151,7 @@ fn magisk_log_write(prio: i32, msg: &[u8]) { Some(s) => s, }; - let result = do_magisk_log_write(logd, prio, msg); + let result = write_log_to_pipe(logd, prio, msg); // If any error occurs, shut down the logd pipe if result.is_err() { @@ -159,7 +159,7 @@ fn magisk_log_write(prio: i32, msg: &[u8]) { } } -fn zygisk_log_write(prio: i32, msg: &[u8]) { +fn zygisk_log_to_pipe(prio: i32, msg: &Utf8CStr) { let magiskd = match MAGISKD.get() { None => return, Some(s) => s, @@ -191,7 +191,7 @@ fn zygisk_log_write(prio: i32, msg: &[u8]) { pthread_sigmask(SIG_BLOCK, &mask, &mut orig_mask); } - let result = do_magisk_log_write(logd, prio, msg); + let result = write_log_to_pipe(logd, prio, msg); // Consume SIGPIPE if exists, then restore mask unsafe { diff --git a/native/src/init/logging.rs b/native/src/init/logging.rs index cc92c9c4e..e845da0e7 100644 --- a/native/src/init/logging.rs +++ b/native/src/init/logging.rs @@ -1,6 +1,6 @@ use std::fs; use std::fs::File; -use std::io::Write; +use std::io::{IoSlice, Write}; use std::sync::OnceLock; use base::ffi::LogLevel; @@ -8,7 +8,7 @@ use base::libc::{ close, makedev, mknod, open, syscall, unlink, SYS_dup3, O_CLOEXEC, O_RDWR, STDERR_FILENO, STDIN_FILENO, STDOUT_FILENO, S_IFCHR, }; -use base::*; +use base::{cstr, exit_on_error, raw_cstr, Logger, Utf8CStr, LOGGER}; static KMSG: OnceLock = OnceLock::new(); @@ -49,19 +49,16 @@ pub fn setup_klog() { writeln!(rate, "on").ok(); } - fn klog_write_impl(_: LogLevel, msg: &[u8]) { + fn kmsg_log_write(_: LogLevel, msg: &Utf8CStr) { if let Some(kmsg) = KMSG.get().as_mut() { - let mut buf = Utf8CStrArr::default(); - buf.append("magiskinit: "); - unsafe { - buf.append_unchecked(msg); - } - kmsg.write_all(buf.as_bytes()).ok(); + let io1 = IoSlice::new("magiskinit: ".as_bytes()); + let io2 = IoSlice::new(msg.as_bytes()); + kmsg.write_vectored(&[io1, io2]).ok(); } } let logger = Logger { - write: klog_write_impl, + write: kmsg_log_write, flags: 0, }; exit_on_error(false);