From 35b3c8ba5c223be2cc8f7798348dcac5725469d8 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Wed, 6 Sep 2023 15:52:14 -0700 Subject: [PATCH] Cleanup persist props code --- native/src/base/misc.rs | 31 +++-- native/src/core/lib.rs | 19 +-- native/src/core/resetprop.rs | 4 + native/src/core/resetprop/mod.rs | 2 - native/src/core/resetprop/persist.rs | 156 +++++++++++------------- native/src/core/resetprop/resetprop.cpp | 8 +- native/src/core/resetprop/resetprop.hpp | 9 +- 7 files changed, 116 insertions(+), 113 deletions(-) create mode 100644 native/src/core/resetprop.rs delete mode 100644 native/src/core/resetprop/mod.rs diff --git a/native/src/base/misc.rs b/native/src/base/misc.rs index 534b21078..19802db7e 100644 --- a/native/src/base/misc.rs +++ b/native/src/base/misc.rs @@ -112,7 +112,25 @@ pub enum StrErr { NullPointerError, } +pub trait StringExt { + fn nul_terminate(&mut self) -> &mut [u8]; +} + +impl StringExt for String { + fn nul_terminate(&mut self) -> &mut [u8] { + self.reserve(1); + // SAFETY: the string is reserved to have enough capacity to fit in the null byte + // SAFETY: the null byte is explicitly added outside of the string's length + unsafe { + let buf = slice::from_raw_parts_mut(self.as_mut_ptr(), self.len() + 1); + *buf.get_unchecked_mut(self.len()) = b'\0'; + buf + } + } +} + // The better CStr: UTF-8 validated + null terminated buffer +#[derive(PartialEq)] pub struct Utf8CStr { inner: [u8], } @@ -129,16 +147,9 @@ impl Utf8CStr { } pub fn from_string(s: &mut String) -> &Utf8CStr { - if s.capacity() == s.len() { - s.reserve(1); - } - // SAFETY: the string is reserved to have enough capacity to fit in the null byte - // SAFETY: the null byte is explicitly added outside of the string's length - unsafe { - let buf = slice::from_raw_parts_mut(s.as_mut_ptr(), s.len() + 1); - *buf.get_unchecked_mut(s.len()) = b'\0'; - Self::from_bytes_unchecked(buf) - } + let buf = s.nul_terminate(); + // SAFETY: the null byte is explicitly added to the buffer + unsafe { Self::from_bytes_unchecked(buf) } } #[inline] diff --git a/native/src/core/lib.rs b/native/src/core/lib.rs index e523f32e0..5d5f2224b 100644 --- a/native/src/core/lib.rs +++ b/native/src/core/lib.rs @@ -5,9 +5,7 @@ use base::Utf8CStr; use cert::read_certificate; use daemon::{daemon_entry, find_apk_path, get_magiskd, zygisk_entry, MagiskD}; use logging::{android_logging, magisk_logging, zygisk_logging}; -use resetprop::persist::{ - persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop, -}; +use resetprop::{persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop}; mod cert; #[path = "../include/consts.rs"] @@ -19,12 +17,15 @@ mod resetprop; #[cxx::bridge] pub mod ffi { extern "C++" { - pub type prop_cb; include!("resetprop/resetprop.hpp"); + + #[cxx_name = "prop_cb"] + type PropCb; + unsafe fn get_prop_rs(name: *const c_char, persist: bool) -> String; + unsafe fn prop_cb_exec(cb: Pin<&mut PropCb>, name: *const c_char, value: *const c_char); + include!("../base/files.hpp"); - pub unsafe fn get_prop_rs(name: *const c_char, persist: bool) -> String; - pub unsafe fn prop_cb_exec(cb: *mut prop_cb, name: *const c_char, value: *const c_char); - pub unsafe fn clone_attr(src: *const c_char, dst: *const c_char); + unsafe fn clone_attr(src: *const c_char, dst: *const c_char); } extern "Rust" { @@ -34,8 +35,8 @@ pub mod ffi { fn zygisk_logging(); fn find_apk_path(pkg: &[u8], data: &mut [u8]) -> usize; fn read_certificate(fd: i32, version: i32) -> Vec; - unsafe fn persist_get_prop(name: *const c_char, prop_cb: *mut prop_cb); - unsafe fn persist_get_props(prop_cb: *mut prop_cb); + unsafe fn persist_get_prop(name: *const c_char, prop_cb: Pin<&mut PropCb>); + unsafe fn persist_get_props(prop_cb: Pin<&mut PropCb>); unsafe fn persist_delete_prop(name: *const c_char) -> bool; unsafe fn persist_set_prop(name: *const c_char, value: *const c_char) -> bool; } diff --git a/native/src/core/resetprop.rs b/native/src/core/resetprop.rs new file mode 100644 index 000000000..bf4fa863a --- /dev/null +++ b/native/src/core/resetprop.rs @@ -0,0 +1,4 @@ +pub use persist::{persist_delete_prop, persist_get_prop, persist_get_props, persist_set_prop}; + +mod persist; +mod proto; diff --git a/native/src/core/resetprop/mod.rs b/native/src/core/resetprop/mod.rs deleted file mode 100644 index 6ab80a2cf..000000000 --- a/native/src/core/resetprop/mod.rs +++ /dev/null @@ -1,2 +0,0 @@ -pub mod persist; -pub mod proto; diff --git a/native/src/core/resetprop/persist.rs b/native/src/core/resetprop/persist.rs index fab499a51..2ecb276d5 100644 --- a/native/src/core/resetprop/persist.rs +++ b/native/src/core/resetprop/persist.rs @@ -1,10 +1,5 @@ -use crate::ffi::{clone_attr, prop_cb, prop_cb_exec}; -use crate::resetprop::proto::persistent_properties::{ - mod_PersistentProperties::PersistentPropertyRecord, PersistentProperties, -}; -use base::{cstr, debug, libc::mkstemp, Directory, LoggedResult, MappedFile, Utf8CStr, WalkResult}; use core::ffi::c_char; -use quick_protobuf::{BytesReader, MessageRead, MessageWrite, Writer}; +use std::pin::Pin; use std::{ fs::{read_to_string, remove_file, rename, File}, io::{BufWriter, Write}, @@ -12,6 +7,18 @@ use std::{ path::{Path, PathBuf}, }; +use quick_protobuf::{BytesReader, MessageRead, MessageWrite, Writer}; + +use base::{ + cstr, debug, libc::mkstemp, Directory, LoggedResult, MappedFile, StringExt, Utf8CStr, + WalkResult, +}; + +use crate::ffi::{clone_attr, prop_cb_exec, PropCb}; +use crate::resetprop::proto::persistent_properties::{ + mod_PersistentProperties::PersistentPropertyRecord, PersistentProperties, +}; + macro_rules! PERSIST_PROP_DIR { () => { "/data/property" @@ -24,12 +31,8 @@ macro_rules! PERSIST_PROP { }; } -struct PropCb { - cb: *mut prop_cb, -} - struct MatchNameCb<'a> { - cb: PropCb, + cb: Pin<&'a mut PropCb>, name: &'a Utf8CStr, } @@ -43,17 +46,15 @@ trait PropCbExec { fn exec(&mut self, name: &Utf8CStr, value: &Utf8CStr); } -impl PropCbExec for PropCb { +impl PropCbExec for Pin<&mut PropCb> { fn exec(&mut self, name: &Utf8CStr, value: &Utf8CStr) { - if !self.cb.is_null() { - unsafe { prop_cb_exec(self.cb, name.as_ptr(), value.as_ptr()) } - } + unsafe { prop_cb_exec(self.as_mut(), name.as_ptr(), value.as_ptr()) } } } impl PropCbExec for MatchNameCb<'_> { fn exec(&mut self, name: &Utf8CStr, value: &Utf8CStr) { - if name.as_bytes() == self.name.as_bytes() { + if name == self.name { self.cb.exec(name, value); debug!("resetprop: found prop [{}] = [{}]", name, value); } @@ -108,10 +109,10 @@ fn file_get_prop(name: &Utf8CStr) -> LoggedResult { fn pb_write_props(props: &PersistentProperties) -> LoggedResult<()> { let mut tmp = String::from(concat!(PERSIST_PROP!(), ".XXXXXX")); - let tmp = Utf8CStr::from_string(&mut tmp); + tmp.nul_terminate(); { let f = unsafe { - let fd = mkstemp(tmp.as_ptr() as *mut c_char); + let fd = mkstemp(tmp.as_mut_ptr().cast()); if fd < 0 { return Err(Default::default()); } @@ -121,7 +122,7 @@ fn pb_write_props(props: &PersistentProperties) -> LoggedResult<()> { props.write_message(&mut Writer::new(BufWriter::new(f)))?; } unsafe { - clone_attr(cstr!(PERSIST_PROP!()).as_ptr(), tmp.as_ptr()); + clone_attr(cstr!(PERSIST_PROP!()).as_ptr(), tmp.as_ptr().cast()); } rename(tmp, PERSIST_PROP!())?; Ok(()) @@ -151,82 +152,73 @@ fn file_set_prop(name: &Utf8CStr, value: Option<&Utf8CStr>) -> LoggedResult<()> Ok(()) } -fn do_persist_get_prop(name: &Utf8CStr, mut prop_cb: PropCb) -> LoggedResult<()> { - if check_pb() { - pb_get_prop(&mut MatchNameCb { cb: prop_cb, name }) - } else { - let mut value = file_get_prop(name)?; - prop_cb.exec(name, Utf8CStr::from_string(&mut value)); - debug!("resetprop: found prop [{}] = [{}]", name, value); - Ok(()) - } -} - -fn do_persist_get_props(mut prop_cb: PropCb) -> LoggedResult<()> { - if check_pb() { - pb_get_prop(&mut prop_cb) - } else { - let mut dir = Directory::open(cstr!(PERSIST_PROP_DIR!()))?; - dir.for_all_file(|f| { - if let Ok(name) = Utf8CStr::from_bytes(f.d_name().to_bytes()) { - if let Ok(mut value) = file_get_prop(name) { - prop_cb.exec(name, Utf8CStr::from_string(&mut value)); - } - } - Ok(WalkResult::Continue) - })?; - Ok(()) - } -} - -fn do_persist_delete_prop(name: &Utf8CStr) -> LoggedResult<()> { - if check_pb() { - let mut pp = PersistentProperties { properties: vec![] }; - pb_get_prop(&mut PropCollectCb { - props: &mut pp, - replace_name: Some(name), - replace_value: None, - })?; - pb_write_props(&pp) - } else { - file_set_prop(name, None) - } -} - -fn do_persist_set_prop(name: &Utf8CStr, value: &Utf8CStr) -> LoggedResult<()> { - if check_pb() { - let mut pp = PersistentProperties { properties: vec![] }; - pb_get_prop(&mut PropCollectCb { - props: &mut pp, - replace_name: Some(name), - replace_value: Some(value), - })?; - pb_write_props(&pp) - } else { - file_set_prop(name, Some(value)) - } -} - -pub unsafe fn persist_get_prop(name: *const c_char, prop_cb: *mut prop_cb) { - unsafe fn inner(name: *const c_char, prop_cb: *mut prop_cb) -> LoggedResult<()> { - do_persist_get_prop(Utf8CStr::from_ptr(name)?, PropCb { cb: prop_cb }) +pub unsafe fn persist_get_prop(name: *const c_char, prop_cb: Pin<&mut PropCb>) { + unsafe fn inner(name: *const c_char, mut prop_cb: Pin<&mut PropCb>) -> LoggedResult<()> { + let name = Utf8CStr::from_ptr(name)?; + if check_pb() { + pb_get_prop(&mut MatchNameCb { cb: prop_cb, name }) + } else { + let mut value = file_get_prop(name)?; + prop_cb.exec(name, Utf8CStr::from_string(&mut value)); + debug!("resetprop: found prop [{}] = [{}]", name, value); + Ok(()) + } } inner(name, prop_cb).ok(); } -pub unsafe fn persist_get_props(prop_cb: *mut prop_cb) { - do_persist_get_props(PropCb { cb: prop_cb }).ok(); +pub unsafe fn persist_get_props(prop_cb: Pin<&mut PropCb>) { + fn inner(mut prop_cb: Pin<&mut PropCb>) -> LoggedResult<()> { + if check_pb() { + pb_get_prop(&mut prop_cb) + } else { + let mut dir = Directory::open(cstr!(PERSIST_PROP_DIR!()))?; + dir.for_all_file(|f| { + if let Ok(name) = Utf8CStr::from_bytes(f.d_name().to_bytes()) { + if let Ok(mut value) = file_get_prop(name) { + prop_cb.exec(name, Utf8CStr::from_string(&mut value)); + } + } + Ok(WalkResult::Continue) + })?; + Ok(()) + } + } + inner(prop_cb).ok(); } pub unsafe fn persist_delete_prop(name: *const c_char) -> bool { unsafe fn inner(name: *const c_char) -> LoggedResult<()> { - do_persist_delete_prop(Utf8CStr::from_ptr(name)?) + let name = Utf8CStr::from_ptr(name)?; + if check_pb() { + let mut pp = PersistentProperties { properties: vec![] }; + pb_get_prop(&mut PropCollectCb { + props: &mut pp, + replace_name: Some(name), + replace_value: None, + })?; + pb_write_props(&pp) + } else { + file_set_prop(name, None) + } } inner(name).is_ok() } pub unsafe fn persist_set_prop(name: *const c_char, value: *const c_char) -> bool { unsafe fn inner(name: *const c_char, value: *const c_char) -> LoggedResult<()> { - do_persist_set_prop(Utf8CStr::from_ptr(name)?, Utf8CStr::from_ptr(value)?) + let name = Utf8CStr::from_ptr(name)?; + let value = Utf8CStr::from_ptr(value)?; + if check_pb() { + let mut pp = PersistentProperties { properties: vec![] }; + pb_get_prop(&mut PropCollectCb { + props: &mut pp, + replace_name: Some(name), + replace_value: Some(value), + })?; + pb_write_props(&pp) + } else { + file_set_prop(name, Some(value)) + } } inner(name, value).is_ok() } diff --git a/native/src/core/resetprop/resetprop.cpp b/native/src/core/resetprop/resetprop.cpp index 9685cac4c..f5606e709 100644 --- a/native/src/core/resetprop/resetprop.cpp +++ b/native/src/core/resetprop/resetprop.cpp @@ -6,6 +6,7 @@ #include #include "resetprop.hpp" +#include "../core-rs.hpp" using namespace std; @@ -195,7 +196,7 @@ static StringType get_prop(const char *name, PropFlags flags) { } if (cb.val.empty() && flags.isPersist() && str_starts(name, "persist.")) - persist_get_prop(name, &cb); + persist_get_prop(name, cb); if (cb.val.empty()) LOGD("resetprop: prop [%s] does not exist\n", name); @@ -208,7 +209,7 @@ static void print_props(PropFlags flags) { if (!flags.isPersistOnly()) system_property_foreach(read_prop_with_cb, &collector); if (flags.isPersist()) - persist_get_props(&collector); + persist_get_props(collector); for (auto &[key, val] : list) { const char *v = flags.isContext() ? (__system_property_get_context(key.data()) ?: "") : @@ -258,7 +259,7 @@ struct Initialize { }; static void InitOnce() { - struct Initialize init; + static struct Initialize init; } #define consume_next(val) \ @@ -269,6 +270,7 @@ stop_parse = true; \ int resetprop_main(int argc, char *argv[]) { PropFlags flags; char *argv0 = argv[0]; + set_log_level_state(LogLevel::Debug, false); const char *prop_file = nullptr; const char *prop_to_rm = nullptr; diff --git a/native/src/core/resetprop/resetprop.hpp b/native/src/core/resetprop/resetprop.hpp index 66c88709e..05b297dd7 100644 --- a/native/src/core/resetprop/resetprop.hpp +++ b/native/src/core/resetprop/resetprop.hpp @@ -29,11 +29,6 @@ int delete_prop(const char *name, bool persist = false); int set_prop(const char *name, const char *value, bool skip_svc = false); void load_prop_file(const char *filename, bool skip_svc = false); -inline void prop_cb_exec(prop_cb *cb, const char *name, const char *value) { - cb->exec(name, value); +static inline void prop_cb_exec(prop_cb &cb, const char *name, const char *value) { + cb.exec(name, value); } - -void persist_get_prop(const char *name, prop_cb *prop_cb) noexcept; -void persist_get_props(prop_cb *prop_cb) noexcept; -bool persist_delete_prop(const char *name) noexcept; -bool persist_set_prop(const char *name, const char *value) noexcept;