From 71213cc6f4b3c564ccd1c1218c5e173df7959274 Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Fri, 12 Sep 2025 12:11:39 -0700 Subject: [PATCH] Fix path tracking in module.rs --- native/src/core/module.rs | 170 ++++++++++++++++++++++++-------------- 1 file changed, 107 insertions(+), 63 deletions(-) diff --git a/native/src/core/module.rs b/native/src/core/module.rs index 8b2e43425..13b0e7192 100644 --- a/native/src/core/module.rs +++ b/native/src/core/module.rs @@ -96,46 +96,101 @@ impl Drop for PathTracker<'_> { } } -struct FilePaths<'a> { +// The comments for this struct assume real = "/system/bin" +struct ModulePaths<'a> { real: PathTracker<'a>, - worker: PathTracker<'a>, + module: PathTracker<'a>, module_mnt: PathTracker<'a>, - module_root: PathTracker<'a>, } -impl FilePaths<'_> { - fn append(&mut self, name: &str) -> FilePaths<'_> { - FilePaths { - real: self.real.append(name), - worker: self.worker.append(name), - module_mnt: self.module_mnt.append(name), - module_root: self.module_root.append(name), +impl ModulePaths<'_> { + fn new<'a>( + real: &'a mut dyn Utf8CStrBuf, + module: &'a mut dyn Utf8CStrBuf, + module_mnt: &'a mut dyn Utf8CStrBuf, + ) -> ModulePaths<'a> { + real.append_path("/"); + module.append_path(MODULEROOT); + module_mnt + .append_path(get_magisk_tmp()) + .append_path(MODULEMNT); + ModulePaths { + real: PathTracker::from(real), + module: PathTracker::from(module), + module_mnt: PathTracker::from(module_mnt), } } - fn reborrow(&mut self) -> FilePaths<'_> { - FilePaths { + fn set_module(&mut self, module: &str) -> ModulePaths<'_> { + ModulePaths { real: self.real.reborrow(), - worker: self.worker.reborrow(), - module_mnt: self.module_mnt.reborrow(), - module_root: self.module_root.reborrow(), + module: self.module.append(module), + module_mnt: self.module_mnt.append(module), } } + fn append(&mut self, name: &str) -> ModulePaths<'_> { + ModulePaths { + real: self.real.append(name), + module: self.module.append(name), + module_mnt: self.module_mnt.append(name), + } + } + + // Returns "/system/bin" fn real(&self) -> &Utf8CStr { self.real.path } - fn worker(&self) -> &Utf8CStr { - self.worker.path + // Returns "$MAGISK_TMP/.magisk/modules/{module}/system/bin" + fn module(&self) -> &Utf8CStr { + self.module.path } + // Returns "/data/adb/modules/{module}/system/bin" fn module_mnt(&self) -> &Utf8CStr { self.module_mnt.path } +} - fn module(&self) -> &Utf8CStr { - self.module_root.path +// The comments for this struct assume real = "/system/bin" +struct MountPaths<'a> { + real: PathTracker<'a>, + worker: PathTracker<'a>, +} + +impl MountPaths<'_> { + fn new<'a>(real: &'a mut dyn Utf8CStrBuf, worker: &'a mut dyn Utf8CStrBuf) -> MountPaths<'a> { + real.append_path("/"); + worker.append_path(get_magisk_tmp()).append_path(WORKERDIR); + MountPaths { + real: PathTracker::from(real), + worker: PathTracker::from(worker), + } + } + + fn append(&mut self, name: &str) -> MountPaths<'_> { + MountPaths { + real: self.real.append(name), + worker: self.worker.append(name), + } + } + + fn reborrow(&mut self) -> MountPaths<'_> { + MountPaths { + real: self.real.reborrow(), + worker: self.worker.reborrow(), + } + } + + // Returns "/system/bin" + fn real(&self) -> &Utf8CStr { + self.real.path + } + + // Returns "$MAGISK_TMP/.magisk/worker/system/bin" + fn worker(&self) -> &Utf8CStr { + self.worker.path } } @@ -154,7 +209,7 @@ impl FsNode { } } - fn collect(&mut self, mut paths: FilePaths) -> LoggedResult<()> { + fn collect(&mut self, mut paths: ModulePaths) -> LoggedResult<()> { let FsNode::Directory { children } = self else { return Ok(()); }; @@ -169,6 +224,7 @@ impl FsNode { .or_insert_with(FsNode::new_dir); node.collect(entry_paths)?; } else if entry.is_symlink() { + // Read the link and store its target let mut link = cstr::buf::default(); path.read_link(&mut link)?; children @@ -225,7 +281,7 @@ impl FsNode { } } - fn commit(&mut self, mut path: FilePaths, is_root_dir: bool) -> LoggedResult<()> { + fn commit(&mut self, mut path: MountPaths, is_root_dir: bool) -> LoggedResult<()> { match self { FsNode::Directory { children } => { let mut is_tmpfs = false; @@ -277,7 +333,7 @@ impl FsNode { Ok(()) } - fn commit_tmpfs(&mut self, mut path: FilePaths) -> LoggedResult<()> { + fn commit_tmpfs(&mut self, mut path: MountPaths) -> LoggedResult<()> { match self { FsNode::Directory { children } => { path.worker().mkdirs(0o000)?; @@ -779,26 +835,12 @@ impl MagiskD { fn apply_modules(&self, module_list: &[ModuleInfo]) { let mut system = FsNode::new_dir(); - // Build all the base "prefix" paths - let mut root = cstr::buf::default().join_path("/"); + // Create buffers for paths + let mut buf1 = cstr::buf::dynamic(256); + let mut buf2 = cstr::buf::dynamic(256); + let mut buf3 = cstr::buf::dynamic(256); - let mut module_dir = cstr::buf::default().join_path(MODULEROOT); - - let mut module_mnt = cstr::buf::default() - .join_path(get_magisk_tmp()) - .join_path(MODULEMNT); - - let mut worker = cstr::buf::default() - .join_path(get_magisk_tmp()) - .join_path(WORKERDIR); - - // Create a collection of all relevant paths - let mut root_paths = FilePaths { - real: PathTracker::from(&mut root), - worker: PathTracker::from(&mut worker), - module_mnt: PathTracker::from(&mut module_mnt), - module_root: PathTracker::from(&mut module_dir), - }; + let mut paths = ModulePaths::new(&mut buf1, &mut buf2, &mut buf3); // Step 1: Create virtual filesystem tree // @@ -806,28 +848,27 @@ impl MagiskD { // record the union of all module filesystem trees under each of their /system directory. for info in module_list { - let mut module_paths = root_paths.append(&info.name); - { - // Read props - let prop = module_paths.append("system.prop"); - if prop.module().exists() { - load_prop_file(prop.module()); - } + let mut paths = paths.set_module(&info.name); + + // Read props + let prop = paths.append("system.prop"); + if prop.module().exists() { + load_prop_file(prop.module()); } - { - // Check whether skip mounting - let skip = module_paths.append("skip_mount"); - if skip.module().exists() { - continue; - } + drop(prop); + + // Check whether skip mounting + let skip = paths.append("skip_mount"); + if skip.module().exists() { + continue; } - { - // Double check whether the system folder exists - let sys = module_paths.append("system"); - if sys.module().exists() { - info!("{}: loading module files", &info.name); - system.collect(sys).log_ok(); - } + drop(skip); + + // Double check whether the system folder exists + let sys = paths.append("system"); + if sys.module().exists() { + info!("{}: loading module files", &info.name); + system.collect(sys).log_ok(); } } @@ -874,6 +915,9 @@ impl MagiskD { } roots.insert("system", system); + drop(paths); + let mut paths = MountPaths::new(&mut buf1, &mut buf2); + for (dir, mut root) in roots { // Step 4: Convert virtual filesystem tree into concrete operations // @@ -882,8 +926,8 @@ impl MagiskD { // The "core" of the logic is to decide which directories need to be rebuilt in the // tmpfs worker directory, and real sub-nodes need to be mirrored inside it. - let path = root_paths.append(dir); - root.commit(path, true).log_ok(); + let paths = paths.append(dir); + root.commit(paths, true).log_ok(); } } }