ksud: fix issues found by clippy (#167)

These issues are mostly found by `cargo clippy -- -W clippy::pedantic`.

.github/workflows/build-ksud.yml

@@ -19,6 +19,7 @@ jobs:
         include:
           - target: aarch64-linux-android
           - target: x86_64-linux-android
+          - target: x86_64-pc-windows-gnu # only for build
     uses: ./.github/workflows/ksud.yml
     with:
       target: ${{ matrix.target }}

userspace/ksud/Cargo.toml

@@ -32,9 +32,12 @@ rust-embed = { version = "6.4.2", features = [
     "compression", # must clean build after updating binaries
 ] }
 
-[target.'cfg(target_os = "android")'.dependencies]
+[target.'cfg(unix)'.dependencies]
 sys-mount = "2.0.1"
+# some android specific dependencies which compiles under unix are also listed here for convenience of coding
 android-properties = { version = "0.2.2", features = ["bionic-deprecated"] }
+
+[target.'cfg(target_os = "android")'.dependencies]
 android_logger = "0.12"
 
 [profile.release]

userspace/ksud/src/apk_sign.rs

@@ -1,6 +1,5 @@
-use std::io::{Read, Seek, SeekFrom};
-
 use anyhow::{ensure, Result};
+use std::io::{Read, Seek, SeekFrom};
 
 pub fn get_apk_signature(apk: &str) -> Result<(u32, u32)> {
     let mut buffer = [0u8; 0x10];
@@ -17,11 +16,11 @@ pub fn get_apk_signature(apk: &str) -> Result<(u32, u32)> {
         f.read_exact(&mut n)?;
 
         let n = u16::from_le_bytes(n);
-        if n as i64 == i {
+        if i64::from(n) == i {
             f.seek(SeekFrom::Current(-22))?;
             f.read_exact(&mut size4)?;
 
-            if u32::from_le_bytes(size4) ^ 0xcafebabeu32 == 0xccfbf1eeu32 {
+            if u32::from_le_bytes(size4) ^ 0xcafe_babe_u32 == 0xccfb_f1ee_u32 {
                 if i > 0 {
                     println!("warning: comment length is {i}");
                 }
@@ -37,14 +36,14 @@ pub fn get_apk_signature(apk: &str) -> Result<(u32, u32)> {
     f.seek(SeekFrom::Current(12))?;
     // offset
     f.read_exact(&mut size4)?;
-    f.seek(SeekFrom::Start(u32::from_le_bytes(size4) as u64 - 0x18))?;
+    f.seek(SeekFrom::Start(u64::from(u32::from_le_bytes(size4)) - 0x18))?;
 
     f.read_exact(&mut size8)?;
     f.read_exact(&mut buffer)?;
 
     ensure!(&buffer == b"APK Sig Block 42", "Can not found sig block");
 
-    let pos = u32::from_le_bytes(size4) as u64 - (u64::from_le_bytes(size8) + 0x8);
+    let pos = u64::from(u32::from_le_bytes(size4)) - (u64::from_le_bytes(size8) + 0x8);
     f.seek(SeekFrom::Start(pos))?;
     f.read_exact(&mut size_of_block)?;
 
@@ -62,7 +61,7 @@ pub fn get_apk_signature(apk: &str) -> Result<(u32, u32)> {
         f.read_exact(&mut id)?; // id
 
         let id = u32::from_le_bytes(id);
-        if (id ^ 0xdeadbeefu32) == 0xafa439f5u32 || (id ^ 0xdeadbeefu32) == 0x2efed62fu32 {
+        if (id ^ 0xdead_beef_u32) == 0xafa4_39f5_u32 || (id ^ 0xdead_beef_u32) == 0x2efe_d62f_u32 {
             f.read_exact(&mut size4)?; // signer-sequence length
             f.read_exact(&mut size4)?; // signer length
             f.read_exact(&mut size4)?; // signed data length
@@ -70,7 +69,7 @@ pub fn get_apk_signature(apk: &str) -> Result<(u32, u32)> {
 
             f.read_exact(&mut size4)?; // digests-sequcence length
             let pos = u32::from_le_bytes(size4);
-            f.seek(SeekFrom::Current(pos as i64))?;
+            f.seek(SeekFrom::Current(i64::from(pos)))?;
             // offset += 0x4 + pos;
 
             f.read_exact(&mut size4)?; // certificates length
@@ -83,18 +82,20 @@ pub fn get_apk_signature(apk: &str) -> Result<(u32, u32)> {
             let j = u32::from_le_bytes(size4);
             for _ in 0..j {
                 f.read_exact(&mut c)?;
-                hash = hash.wrapping_mul(31).wrapping_add(c[0] as i8 as i32);
+                hash = hash.wrapping_mul(31).wrapping_add(i32::from(c[0] as i8));
             }
 
             // offset += j;
 
             let out_size = j;
-            let out_hash = (hash as u32) ^ 0x14131211u32;
+            let out_hash = (hash as u32) ^ 0x1413_1211_u32;
 
             return Ok((out_size, out_hash));
         }
 
-        f.seek(SeekFrom::Current(i64::from_le_bytes(size8) - offset as i64))?;
+        f.seek(SeekFrom::Current(
+            i64::from_le_bytes(size8) - i64::from(offset),
+        ))?;
     }
 
     Err(anyhow::anyhow!("Unknown error"))

userspace/ksud/src/cli.rs

@@ -1,7 +1,9 @@
 use anyhow::{Ok, Result};
 use clap::Parser;
-#[cfg(target_os="android")]
+
+#[cfg(target_os = "android")]
 use android_logger::Config;
+#[cfg(target_os = "android")]
 use log::LevelFilter;
 
 use crate::{apk_sign, debug, event, module};
@@ -120,14 +122,14 @@ enum Module {
 }
 
 pub fn run() -> Result<()> {
-    #[cfg(target_os="android")]
+    #[cfg(target_os = "android")]
     android_logger::init_once(
         Config::default()
             .with_max_level(LevelFilter::Trace) // limit log level
-            .with_tag("KernelSU") // logs will show under mytag tag
+            .with_tag("KernelSU"), // logs will show under mytag tag
     );
 
-    #[cfg(not(target_os="android"))]
+    #[cfg(not(target_os = "android"))]
     env_logger::init();
 
     let cli = Args::parse();
@@ -139,16 +141,13 @@ pub fn run() -> Result<()> {
         Commands::PostFsData => event::on_post_data_fs(),
         Commands::BootCompleted => event::on_boot_completed(),
 
-        Commands::Module { command } => {
+        Commands::Module { command } => match command {
-
+            Module::Install { zip } => module::install_module(&zip),
-            match command {
+            Module::Uninstall { id } => module::uninstall_module(&id),
-                Module::Install { zip } => module::install_module(zip),
+            Module::Enable { id } => module::enable_module(&id),
-                Module::Uninstall { id } => module::uninstall_module(id),
+            Module::Disable { id } => module::disable_module(&id),
-                Module::Enable { id } => module::enable_module(id),
+            Module::List => module::list_modules(),
-                Module::Disable { id } => module::disable_module(id),
+        },
-                Module::List => module::list_modules(),
-            }
-        }
         Commands::Install => event::install(),
         Commands::Sepolicy { command } => match command {
             Sepolicy::Patch { sepolicy } => crate::sepolicy::live_patch(&sepolicy),
@@ -168,7 +167,7 @@ pub fn run() -> Result<()> {
                 Ok(())
             }
             Debug::Su => crate::ksu::grant_root(),
-            Debug::Test => todo!()
+            Debug::Test => todo!(),
         },
     };
 

userspace/ksud/src/debug.rs

@@ -55,7 +55,7 @@ pub fn set_manager(pkg: &str) -> Result<()> {
     );
 
     let path = get_apk_path(pkg).with_context(|| format!("{pkg} does not exist!"))?;
-    let sign = get_apk_signature(path.as_str())?;
+    let sign = get_apk_signature(&path)?;
     set_kernel_param(sign.0, sign.1)?;
     Ok(())
 }

userspace/ksud/src/event.rs

@@ -1,11 +1,11 @@
+use anyhow::{bail, Context, Result};
+use log::{info, warn};
 use std::{collections::HashMap, path::Path};
 
 use crate::{
     assets, defs, mount,
     utils::{ensure_clean_dir, ensure_dir_exists},
 };
-use anyhow::{bail, Context, Result};
-use log::{info, warn};
 
 fn mount_partition(partition: &str, lowerdir: &mut Vec<String>) -> Result<()> {
     if lowerdir.is_empty() {
@@ -28,7 +28,7 @@ fn mount_partition(partition: &str, lowerdir: &mut Vec<String>) -> Result<()> {
     mount::mount_overlay(&lowerdir, &lowest_dir)
 }
 
-pub fn do_systemless_mount(module_dir: &str) -> Result<()> {
+pub fn mount_systemlessly(module_dir: &str) -> Result<()> {
     // construct overlay mount params
     let dir = std::fs::read_dir(module_dir);
     let Ok(dir) = dir else {
@@ -40,7 +40,7 @@ pub fn do_systemless_mount(module_dir: &str) -> Result<()> {
     let partition = vec!["vendor", "product", "system_ext", "odm", "oem"];
     let mut partition_lowerdir: HashMap<String, Vec<String>> = HashMap::new();
     for ele in &partition {
-        partition_lowerdir.insert(ele.to_string(), Vec::new());
+        partition_lowerdir.insert((*ele).to_string(), Vec::new());
     }
 
     for entry in dir.flatten() {
@@ -55,7 +55,7 @@ pub fn do_systemless_mount(module_dir: &str) -> Result<()> {
         }
 
         let module_system = Path::new(&module).join("system");
-        if !module_system.as_path().exists() {
+        if !module_system.exists() {
             info!("module: {} has no system overlay.", module.display());
             continue;
         }
@@ -133,12 +133,14 @@ pub fn on_post_data_fs() -> Result<()> {
     }
 
     // mount systemless overlay
-    if let Err(e) = do_systemless_mount(module_dir) {
+    if let Err(e) = mount_systemlessly(module_dir) {
         warn!("do systemless mount failed: {}", e);
     }
 
     // module mounted, exec modules post-fs-data scripts
-    if !crate::utils::is_safe_mode() {
+    if crate::utils::is_safe_mode() {
+        warn!("safe mode, skip module post-fs-data scripts");
+    } else {
         // todo: Add timeout
         if let Err(e) = crate::module::exec_common_scripts("post-fs-data.d", true) {
             warn!("exec common post-fs-data scripts failed: {}", e);
@@ -149,8 +151,6 @@ pub fn on_post_data_fs() -> Result<()> {
         if let Err(e) = crate::module::load_system_prop() {
             warn!("load system.prop failed: {}", e);
         }
-    } else {
-        warn!("safe mode, skip module post-fs-data scripts");
     }
 
     Ok(())
@@ -158,15 +158,15 @@ pub fn on_post_data_fs() -> Result<()> {
 
 pub fn on_services() -> Result<()> {
     // exec modules service.sh scripts
-    if !crate::utils::is_safe_mode() {
+    if crate::utils::is_safe_mode() {
+        warn!("safe mode, skip module service scripts");
+    } else {
         if let Err(e) = crate::module::exec_common_scripts("service.d", false) {
             warn!("exec common service scripts failed: {}", e);
         }
         if let Err(e) = crate::module::exec_services() {
             warn!("exec service scripts failed: {}", e);
         }
-    } else {
-        warn!("safe mode, skip module service scripts");
     }
 
     Ok(())

userspace/ksud/src/ksu.rs

@@ -1,9 +1,11 @@
-#![allow(dead_code, unused_mut, unused_variables, unused_imports)]
+use anyhow::Result;
 
-use anyhow::{ensure, Result};
+#[cfg(unix)]
+use anyhow::ensure;
+#[cfg(unix)]
 use std::os::unix::process::CommandExt;
 
-pub const KERNEL_SU_OPTION: u32 = 0xDEADBEEF;
+pub const KERNEL_SU_OPTION: u32 = 0xDEAD_BEEF;
 
 const CMD_GRANT_ROOT: u64 = 0;
 // const CMD_BECOME_MANAGER: u64 = 1;
@@ -18,45 +20,52 @@ pub const CMD_SET_SEPOLICY: u64 = 8;
 const EVENT_POST_FS_DATA: u64 = 1;
 const EVENT_BOOT_COMPLETED: u64 = 2;
 
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 pub fn grant_root() -> Result<()> {
     let mut result: u32 = 0;
     unsafe {
+        #[allow(clippy::cast_possible_wrap)]
         libc::prctl(
-            KERNEL_SU_OPTION as i32,
+            KERNEL_SU_OPTION as i32, // supposed to overflow
             CMD_GRANT_ROOT,
             0,
             0,
-            &mut result as *mut _ as *mut libc::c_void,
+            std::ptr::addr_of_mut!(result).cast::<libc::c_void>(),
         );
     }
 
     ensure!(result == KERNEL_SU_OPTION, "grant root failed");
-    return Err(std::process::Command::new("sh").exec().into());
+    Err(std::process::Command::new("sh").exec().into())
 }
 
-#[cfg(not(target_os = "android"))]
+#[cfg(not(unix))]
 pub fn grant_root() -> Result<()> {
     unimplemented!("grant_root is only available on android");
 }
 
 pub fn get_version() -> i32 {
     let mut result: i32 = 0;
-    #[cfg(target_os = "android")]
+    #[cfg(unix)]
     unsafe {
+        #[allow(clippy::cast_possible_wrap)]
         libc::prctl(
-            KERNEL_SU_OPTION as i32,
+            KERNEL_SU_OPTION as i32, // supposed to overflow
             CMD_GET_VERSION,
-            &mut result as *mut _ as *mut libc::c_void,
+            std::ptr::addr_of_mut!(result).cast::<libc::c_void>(),
         );
     }
     result
 }
 
 fn report_event(event: u64) {
-    #[cfg(target_os = "android")]
+    #[cfg(unix)]
     unsafe {
-        libc::prctl(KERNEL_SU_OPTION as i32, CMD_REPORT_EVENT, event);
+        #[allow(clippy::cast_possible_wrap)]
+        libc::prctl(
+            KERNEL_SU_OPTION as i32, // supposed to overflow
+            CMD_REPORT_EVENT,
+            event,
+        );
     }
 }
 

userspace/ksud/src/main.rs

@@ -1,15 +1,15 @@
 mod apk_sign;
+mod assets;
 mod cli;
 mod debug;
 mod defs;
 mod event;
 mod ksu;
 mod module;
-mod restorecon;
-mod utils;
-mod sepolicy;
-mod assets;
 mod mount;
+mod restorecon;
+mod sepolicy;
+mod utils;
 
 fn main() -> anyhow::Result<()> {
     cli::run()

userspace/ksud/src/module.rs

@@ -1,26 +1,29 @@
+#[allow(clippy::wildcard_imports)]
+use crate::utils::*;
 use crate::{
     assets, defs, mount,
     restorecon::{restore_syscon, setsyscon},
     sepolicy,
-    utils::*,
 };
 
+use anyhow::{anyhow, bail, ensure, Context, Result};
 use const_format::concatcp;
+use is_executable::is_executable;
 use java_properties::PropertiesIter;
-use log::{debug, info, warn};
+use log::{info, warn};
 use std::{
     collections::HashMap,
     env::var as env_var,
     fs::{remove_dir_all, set_permissions, File, OpenOptions, Permissions},
-    io::{Cursor, Read, Write},
+    io::{Cursor, Write},
-    os::unix::{prelude::PermissionsExt, process::CommandExt},
     path::{Path, PathBuf},
     process::{Command, Stdio},
     str::FromStr,
 };
 use zip_extensions::zip_extract_file_to_memory;
 
-use anyhow::{bail, ensure, Context, Result};
+#[cfg(unix)]
+use std::os::unix::{prelude::PermissionsExt, process::CommandExt};
 
 const UTIL_FUNCTIONS: &str = include_str!("./installer.sh");
 const INSTALL_MODULE_SCRIPT: &str =
@@ -161,17 +164,6 @@ fn switch_cgroups() {
     }
 }
 
-fn is_executable(path: &Path) -> bool {
-    let mut buffer = [0u8; 2];
-    is_executable::is_executable(path)
-        && File::open(path).unwrap().read_exact(&mut buffer).is_ok()
-        && (
-            buffer == [0x23, 0x21] // shebang #!
-            || buffer == [0x7f, 0x45]
-            // ELF magic number 0x7F 'E'
-        )
-}
-
 pub fn load_sepolicy_rule() -> Result<()> {
     let modules_dir = Path::new(defs::MODULE_DIR);
     let dir = std::fs::read_dir(modules_dir)?;
@@ -197,6 +189,40 @@ pub fn load_sepolicy_rule() -> Result<()> {
     Ok(())
 }
 
+fn exec_script<T: AsRef<Path>>(path: T, wait: bool) -> Result<()> {
+    info!("exec {}", path.as_ref().display());
+
+    let mut command = &mut Command::new(assets::BUSYBOX_PATH);
+    #[cfg(unix)]
+    {
+        command = command.process_group(0);
+        command = unsafe {
+            command.pre_exec(|| {
+                // ignore the error?
+                switch_cgroups();
+                Ok(())
+            })
+        };
+    }
+    command = command
+        .current_dir(path.as_ref().parent().unwrap())
+        .arg("sh")
+        .arg(path.as_ref())
+        .env("ASH_STANDALONE", "1")
+        .env(
+            "PATH",
+            format!("{}:{}", env_var("PATH").unwrap(), defs::BINARY_DIR),
+        )
+        .env("KSU", "true");
+
+    let result = if wait {
+        command.status().map(|_| ())
+    } else {
+        command.spawn().map(|_| ())
+    };
+    result.map_err(|err| anyhow!("Failed to exec {}: {}", path.as_ref().display(), err))
+}
+
 /// execute every modules' post-fs-data.sh
 pub fn exec_post_fs_data() -> Result<()> {
     let modules_dir = Path::new(defs::MODULE_DIR);
@@ -213,31 +239,8 @@ pub fn exec_post_fs_data() -> Result<()> {
         if !post_fs_data.exists() {
             continue;
         }
-        info!("exec {} post-fs-data.sh", path.display());
 
-        let mut command = Command::new(assets::BUSYBOX_PATH);
+        exec_script(path, true)?;
-        let command = command.arg("sh");
-        let command = command.arg(&post_fs_data);
-
-        let command = command
-            .process_group(0)
-            .current_dir(path)
-            .env("ASH_STANDALONE", "1")
-            .env(
-                "PATH",
-                format!("{}:{}", env_var("PATH").unwrap(), defs::BINARY_DIR),
-            )
-            .env("KSU", "true");
-        let command = unsafe {
-            command.pre_exec(|| {
-                // ignore the error?
-                switch_cgroups();
-                Ok(())
-            })
-        };
-        command
-            .status()
-            .with_context(|| format!("Failed to exec {}", post_fs_data.display()))?;
     }
 
     Ok(())
@@ -263,37 +266,7 @@ pub fn exec_common_scripts(dir: &str, wait: bool) -> Result<()> {
             continue;
         }
 
-        info!("exec {}", path.display());
+        exec_script(path, wait)?;
-
-        let mut command = Command::new(assets::BUSYBOX_PATH);
-        let command = command.arg("sh");
-        let command = command.arg(&path);
-        let command = command
-            .process_group(0)
-            .current_dir(&script_dir)
-            .env("ASH_STANDALONE", "1")
-            .env(
-                "PATH",
-                format!("{}:{}", env_var("PATH").unwrap(), defs::BINARY_DIR),
-            )
-            .env("KSU", "true");
-
-        let command = unsafe {
-            command.pre_exec(|| {
-                switch_cgroups();
-                Ok(())
-            })
-        };
-
-        if !wait {
-            command
-                .spawn() // don't wait
-                .with_context(|| format!("Failed to exec {}", path.display()))?;
-        } else {
-            command
-                .status()
-                .with_context(|| format!("Failed to exec {}", path.display()))?;
-        }
     }
 
     Ok(())
@@ -315,31 +288,8 @@ pub fn exec_services() -> Result<()> {
         if !service.exists() {
             continue;
         }
-        info!("exec {} service.sh", path.display());
 
-        let mut command = Command::new(assets::BUSYBOX_PATH);
+        exec_script(path, false)?;
-        let command = command.arg("sh");
-        let command = command.arg(&service);
-
-        let command = command
-            .process_group(0)
-            .current_dir(path)
-            .env("ASH_STANDALONE", "1")
-            .env(
-                "PATH",
-                format!("{}:{}", env_var("PATH").unwrap(), defs::BINARY_DIR),
-            )
-            .env("KSU", "true");
-        let command = unsafe {
-            command.pre_exec(|| {
-                // ignore the error?
-                switch_cgroups();
-                Ok(())
-            })
-        };
-        command
-            .spawn() // don't wait
-            .with_context(|| format!("Failed to exec {}", service.display()))?;
     }
 
     Ok(())
@@ -374,7 +324,7 @@ pub fn load_system_prop() -> Result<()> {
     Ok(())
 }
 
-fn do_install_module(zip: String) -> Result<()> {
+fn _install_module(zip: &str) -> Result<()> {
     ensure_boot_completed()?;
 
     // print banner
@@ -389,7 +339,7 @@ fn do_install_module(zip: String) -> Result<()> {
     // read the module_id from zip, if faild if will return early.
     let mut buffer: Vec<u8> = Vec::new();
     let entry_path = PathBuf::from_str("module.prop")?;
-    let zip_path = PathBuf::from_str(&zip)?;
+    let zip_path = PathBuf::from_str(zip)?;
     zip_extract_file_to_memory(&zip_path, &entry_path, &mut buffer)?;
 
     let mut module_prop = HashMap::new();
@@ -419,7 +369,7 @@ fn do_install_module(zip: String) -> Result<()> {
     }
 
     let default_reserve_size = 64 * 1024 * 1024;
-    let zip_uncompressed_size = get_zip_uncompressed_size(&zip)?;
+    let zip_uncompressed_size = get_zip_uncompressed_size(zip)?;
     let grow_size = default_reserve_size + zip_uncompressed_size;
 
     info!(
@@ -501,16 +451,17 @@ fn do_install_module(zip: String) -> Result<()> {
     info!("module dir: {}", module_dir);
 
     // unzip the image and move it to modules_update/<id> dir
-    let file = File::open(&zip)?;
+    let file = File::open(zip)?;
     let mut archive = zip::ZipArchive::new(file)?;
     archive.extract(&module_dir)?;
 
-    exec_install_script(&zip)?;
+    exec_install_script(zip)?;
 
     // set permission and selinux context for $MOD/system
     let module_system_dir = PathBuf::from(module_dir).join("system");
     if module_system_dir.exists() {
         let path = module_system_dir.to_str().unwrap();
+        #[cfg(unix)]
         set_permissions(&module_system_dir, Permissions::from_mode(0o755))?;
         restore_syscon(path)?;
     }
@@ -531,8 +482,8 @@ fn do_install_module(zip: String) -> Result<()> {
     Ok(())
 }
 
-pub fn install_module(zip: String) -> Result<()> {
+pub fn install_module(zip: &str) -> Result<()> {
-    let result = do_install_module(zip);
+    let result = _install_module(zip);
     if let Err(ref e) = result {
         // error happened, do some cleanup!
         let _ = std::fs::remove_file(defs::MODULE_UPDATE_TMP_IMG);
@@ -542,7 +493,7 @@ pub fn install_module(zip: String) -> Result<()> {
     result
 }
 
-fn do_module_update<F>(update_dir: &str, id: &str, func: F) -> Result<()>
+fn update_module<F>(update_dir: &str, id: &str, func: F) -> Result<()>
 where
     F: Fn(&str, &str) -> Result<()>,
 {
@@ -587,8 +538,8 @@ where
     result
 }
 
-pub fn uninstall_module(id: String) -> Result<()> {
+pub fn uninstall_module(id: &str) -> Result<()> {
-    do_module_update(defs::MODULE_UPDATE_TMP_DIR, &id, |mid, update_dir| {
+    update_module(defs::MODULE_UPDATE_TMP_DIR, id, |mid, update_dir| {
         let dir = Path::new(update_dir);
         ensure!(dir.exists(), "No module installed");
 
@@ -621,13 +572,13 @@ pub fn uninstall_module(id: String) -> Result<()> {
             remove_dir_all(target_module)?;
         }
 
-        let _ = mark_module_state(&id, defs::REMOVE_FILE_NAME, true);
+        let _ = mark_module_state(id, defs::REMOVE_FILE_NAME, true);
 
         Ok(())
     })
 }
 
-fn do_enable_module(module_dir: &str, mid: &str, enable: bool) -> Result<()> {
+fn _enable_module(module_dir: &str, mid: &str, enable: bool) -> Result<()> {
     let src_module_path = format!("{module_dir}/{mid}");
     let src_module = Path::new(&src_module_path);
     ensure!(src_module.exists(), "module: {} not found!", mid);
@@ -648,19 +599,19 @@ fn do_enable_module(module_dir: &str, mid: &str, enable: bool) -> Result<()> {
     Ok(())
 }
 
-pub fn enable_module(id: String) -> Result<()> {
+pub fn enable_module(id: &str) -> Result<()> {
-    do_module_update(defs::MODULE_UPDATE_TMP_DIR, &id, |mid, update_dir| {
+    update_module(defs::MODULE_UPDATE_TMP_DIR, id, |mid, update_dir| {
-        do_enable_module(update_dir, mid, true)
+        _enable_module(update_dir, mid, true)
     })
 }
 
-pub fn disable_module(id: String) -> Result<()> {
+pub fn disable_module(id: &str) -> Result<()> {
-    do_module_update(defs::MODULE_UPDATE_TMP_DIR, &id, |mid, update_dir| {
+    update_module(defs::MODULE_UPDATE_TMP_DIR, id, |mid, update_dir| {
-        do_enable_module(update_dir, mid, false)
+        _enable_module(update_dir, mid, false)
     })
 }
 
-fn do_list_modules(path: &str) -> Vec<HashMap<String, String>> {
+fn _list_modules(path: &str) -> Vec<HashMap<String, String>> {
     // first check enabled modules
     let dir = std::fs::read_dir(path);
     let Ok(dir) = dir else {
@@ -681,7 +632,7 @@ fn do_list_modules(path: &str) -> Vec<HashMap<String, String>> {
             warn!("Failed to read file: {}", module_prop.display());
             continue;
         };
-        let mut module_prop_map = HashMap::new();
+        let mut module_prop_map: HashMap<String, String> = HashMap::new();
         let encoding = encoding::all::UTF_8;
         let result =
             PropertiesIter::new_with_encoding(Cursor::new(content), encoding).read_into(|k, v| {
@@ -693,9 +644,9 @@ fn do_list_modules(path: &str) -> Vec<HashMap<String, String>> {
         let update = path.join(defs::UPDATE_FILE_NAME).exists();
         let remove = path.join(defs::REMOVE_FILE_NAME).exists();
 
-        module_prop_map.insert("enabled".to_string(), enabled.to_string());
+        module_prop_map.insert("enabled".to_owned(), enabled.to_string());
-        module_prop_map.insert("update".to_string(), update.to_string());
+        module_prop_map.insert("update".to_owned(), update.to_string());
-        module_prop_map.insert("remove".to_string(), remove.to_string());
+        module_prop_map.insert("remove".to_owned(), remove.to_string());
 
         if result.is_err() {
             warn!("Failed to parse module.prop: {}", module_prop.display());
@@ -708,7 +659,7 @@ fn do_list_modules(path: &str) -> Vec<HashMap<String, String>> {
 }
 
 pub fn list_modules() -> Result<()> {
-    let modules = do_list_modules(defs::MODULE_DIR);
+    let modules = _list_modules(defs::MODULE_DIR);
     println!("{}", serde_json::to_string_pretty(&modules)?);
     Ok(())
 }

userspace/ksud/src/mount.rs

@@ -1,23 +1,21 @@
 use anyhow::Result;
 
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 use anyhow::{Context, Ok};
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 use retry::delay::NoDelay;
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 use sys_mount::{unmount, FilesystemType, Mount, MountFlags, Unmount, UnmountFlags};
 
-#[allow(dead_code)]
 pub struct AutoMountExt4 {
     mnt: String,
-    #[cfg(target_os = "android")]
+    #[cfg(unix)]
     mount: Option<Mount>,
     auto_umount: bool,
 }
 
-#[allow(dead_code)]
 impl AutoMountExt4 {
-    #[cfg(target_os = "android")]
+    #[cfg(unix)]
     pub fn try_new(src: &str, mnt: &str, auto_umount: bool) -> Result<Self> {
         let result = Mount::builder()
             .fstype(FilesystemType::from("ext4"))
@@ -27,11 +25,11 @@ impl AutoMountExt4 {
                 Ok(Self {
                     mnt: mnt.to_string(),
                     mount: Some(mount),
-                    auto_umount
+                    auto_umount,
                 })
             });
         if let Err(e) = result {
-            println!("- Mount failed: {}, retry with system mount", e);
+            println!("- Mount failed: {e}, retry with system mount");
             let result = std::process::Command::new("mount")
                 .arg("-t")
                 .arg("ext4")
@@ -46,7 +44,7 @@ impl AutoMountExt4 {
                 Ok(Self {
                     mnt: mnt.to_string(),
                     mount: None,
-                    auto_umount
+                    auto_umount,
                 })
             }
         } else {
@@ -54,41 +52,45 @@ impl AutoMountExt4 {
         }
     }
 
-    #[cfg(not(target_os = "android"))]
+    #[cfg(not(unix))]
     pub fn try_new(_src: &str, _mnt: &str, _auto_umount: bool) -> Result<Self> {
         unimplemented!()
     }
 
-    #[cfg(target_os = "android")]
+    #[cfg(unix)]
     pub fn umount(&self) -> Result<()> {
-        match self.mount {
+        if let Some(ref mount) = self.mount {
-            Some(ref mount) => mount
+            mount
                 .unmount(UnmountFlags::empty())
-                .map_err(|e| anyhow::anyhow!(e)),
+                .map_err(|e| anyhow::anyhow!(e))
-            None => {
+        } else {
-                let result = std::process::Command::new("umount").arg(&self.mnt).status();
+            let result = std::process::Command::new("umount").arg(&self.mnt).status();
-                if let Err(e) = result {
+            if let Err(e) = result {
-                    Err(anyhow::anyhow!("umount: {} failed: {e}", self.mnt))
+                Err(anyhow::anyhow!("umount: {} failed: {e}", self.mnt))
-                } else {
+            } else {
-                    Ok(())
+                Ok(())
-                }
             }
         }
     }
 }
 
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 impl Drop for AutoMountExt4 {
     fn drop(&mut self) {
-        log::info!("AutoMountExt4 drop: {}, auto_umount: {}", self.mnt, self.auto_umount);
+        log::info!(
+            "AutoMountExt4 drop: {}, auto_umount: {}",
+            self.mnt,
+            self.auto_umount
+        );
         if self.auto_umount {
             let _ = self.umount();
         }
     }
 }
 
-#[cfg(target_os = "android")]
+#[allow(dead_code)]
-fn do_mount_image(src: &str, target: &str, autodrop: bool) -> Result<()> {
+#[cfg(unix)]
+fn mount_image(src: &str, target: &str, autodrop: bool) -> Result<()> {
     if autodrop {
         Mount::builder()
             .fstype(FilesystemType::from("ext4"))
@@ -103,23 +105,24 @@ fn do_mount_image(src: &str, target: &str, autodrop: bool) -> Result<()> {
     Ok(())
 }
 
-#[cfg(target_os = "android")]
+#[allow(dead_code)]
+#[cfg(unix)]
 pub fn mount_ext4(src: &str, target: &str, autodrop: bool) -> Result<()> {
     // umount target first.
     let _ = umount_dir(target);
-    let result = retry::retry(NoDelay.take(3), || do_mount_image(src, target, autodrop));
+    let result = retry::retry(NoDelay.take(3), || mount_image(src, target, autodrop));
     result
         .map_err(|e| anyhow::anyhow!("mount partition: {src} -> {target} failed: {e}"))
         .map(|_| ())
 }
 
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 pub fn umount_dir(src: &str) -> Result<()> {
     unmount(src, UnmountFlags::empty()).with_context(|| format!("Failed to umount {src}"))?;
     Ok(())
 }
 
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 pub fn mount_overlay(lowerdir: &str, mnt: &str) -> Result<()> {
     Mount::builder()
         .fstype(FilesystemType::from("overlay"))
@@ -130,17 +133,17 @@ pub fn mount_overlay(lowerdir: &str, mnt: &str) -> Result<()> {
         .map_err(|e| anyhow::anyhow!("mount partition: {mnt} overlay failed: {e}"))
 }
 
-#[cfg(not(target_os = "android"))]
+#[cfg(not(unix))]
 pub fn mount_ext4(_src: &str, _target: &str, _autodrop: bool) -> Result<()> {
     unimplemented!()
 }
 
-#[cfg(not(target_os = "android"))]
+#[cfg(not(unix))]
 pub fn umount_dir(_src: &str) -> Result<()> {
     unimplemented!()
 }
 
-#[cfg(not(target_os = "android"))]
+#[cfg(not(unix))]
 pub fn mount_overlay(_lowerdir: &str, _mnt: &str) -> Result<()> {
     unimplemented!()
 }

userspace/ksud/src/restorecon.rs

@@ -1,33 +1,37 @@
-use anyhow::{Context, Ok, Result};
+use anyhow::Result;
-#[cfg(target_os = "android")]
-use extattr::{setxattr, Flags as XattrFlags};
 use jwalk::{Parallelism::Serial, WalkDir};
 
+#[cfg(unix)]
+use anyhow::{Context, Ok};
+#[cfg(unix)]
+use extattr::{setxattr, Flags as XattrFlags};
+
 const SYSTEM_CON: &str = "u:object_r:system_file:s0";
 const _ADB_CON: &str = "u:object_r:adb_data_file:s0";
 
 pub fn setcon(path: &str, con: &str) -> Result<()> {
-    #[cfg(target_os = "android")]
+    #[cfg(unix)]
     setxattr(path, "security.selinux", con, XattrFlags::empty())
         .with_context(|| format!("Failed to change SELinux context for {path}"))?;
     Ok(())
 }
 
+#[cfg(unix)]
 pub fn setsyscon(path: &str) -> Result<()> {
     setcon(path, SYSTEM_CON)
 }
 
+#[cfg(not(unix))]
+pub fn setsyscon(_path: &str) -> Result<()> {
+    unimplemented!()
+}
+
 pub fn restore_syscon(dir: &str) -> Result<()> {
     for dir_entry in WalkDir::new(dir).parallelism(Serial) {
         if let Some(path) = dir_entry.ok().map(|dir_entry| dir_entry.path()) {
-            #[cfg(target_os = "android")]
+            #[cfg(unix)]
             setxattr(&path, "security.selinux", SYSTEM_CON, XattrFlags::empty()).with_context(
-                || {
+                || format!("Failed to change SELinux context for {}", path.display()),
-                    format!(
-                        "Failed to change SELinux context for {}",
-                        path.to_str().unwrap()
-                    )
-                },
             )?;
         }
     }

userspace/ksud/src/sepolicy.rs

@@ -345,7 +345,7 @@ impl<'a> PolicyStatement<'a> {
     }
 }
 
-fn parse_sepolicy<'a, 'b>(input: &'b str) -> Result<Vec<PolicyStatement<'a>>>
+fn parse_sepolicy<'a, 'b>(input: &'b str) -> Vec<PolicyStatement<'a>>
 where
     'b: 'a,
 {
@@ -356,7 +356,7 @@ where
             statements.push(statement);
         }
     }
-    Ok(statements)
+    statements
 }
 
 const SEPOLICY_MAX_LEN: usize = 128;
@@ -401,6 +401,7 @@ impl TryFrom<&str> for PolicyObject {
 /// normal statement would be expand to atomic statement, for example:
 /// allow domain1 domain2:file1 { read write }; would be expand to two atomic statement
 /// allow domain1 domain2:file1 read;allow domain1 domain2:file1 write;
+#[allow(clippy::too_many_arguments)]
 #[derive(Debug, new)]
 struct AtomicStatement {
     cmd: u32,
@@ -552,8 +553,7 @@ impl<'a> TryFrom<&'a TypeAttr<'a>> for Vec<AtomicStatement> {
 impl<'a> TryFrom<&'a Attr<'a>> for Vec<AtomicStatement> {
     type Error = anyhow::Error;
     fn try_from(perm: &'a Attr<'a>) -> Result<Self> {
-        let mut result = vec![];
+        let result = vec![AtomicStatement {
-        result.push(AtomicStatement {
             cmd: CMD_ATTR,
             subcmd: 0,
             sepol1: perm.name.try_into()?,
@@ -563,7 +563,7 @@ impl<'a> TryFrom<&'a Attr<'a>> for Vec<AtomicStatement> {
             sepol5: PolicyObject::None,
             sepol6: PolicyObject::None,
             sepol7: PolicyObject::None,
-        });
+        }];
         Ok(result)
     }
 }
@@ -670,9 +670,8 @@ struct FfiPolicy {
 
 fn to_c_ptr(pol: &PolicyObject) -> *const libc::c_char {
     match pol {
-        PolicyObject::None => std::ptr::null(),
+        PolicyObject::None | PolicyObject::All => std::ptr::null(),
-        PolicyObject::All => std::ptr::null(),
+        PolicyObject::One(s) => s.as_ptr().cast::<libc::c_char>(),
-        PolicyObject::One(s) => s.as_ptr() as *const libc::c_char,
     }
 }
 
@@ -692,7 +691,7 @@ impl From<AtomicStatement> for FfiPolicy {
     }
 }
 
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 fn apply_one_rule<'a>(statement: &'a PolicyStatement<'a>) -> Result<()> {
     let policies: Vec<AtomicStatement> = statement.try_into()?;
 
@@ -700,12 +699,13 @@ fn apply_one_rule<'a>(statement: &'a PolicyStatement<'a>) -> Result<()> {
         let mut result: u32 = 0;
         let cpolicy = FfiPolicy::from(policy);
         unsafe {
+            #[allow(clippy::cast_possible_wrap)]
             libc::prctl(
-                crate::ksu::KERNEL_SU_OPTION as i32,
+                crate::ksu::KERNEL_SU_OPTION as i32, // supposed to overflow
                 crate::ksu::CMD_SET_SEPOLICY,
                 0,
-                &cpolicy as *const _ as *const libc::c_void,
+                std::ptr::addr_of!(cpolicy).cast::<libc::c_void>(),
-                &mut result as *mut _ as *mut libc::c_void,
+                std::ptr::addr_of_mut!(result).cast::<libc::c_void>(),
             );
         }
 
@@ -717,13 +717,13 @@ fn apply_one_rule<'a>(statement: &'a PolicyStatement<'a>) -> Result<()> {
     Ok(())
 }
 
-#[cfg(not(target_os = "android"))]
+#[cfg(not(unix))]
 fn apply_one_rule<'a>(_statement: &'a PolicyStatement<'a>) -> Result<()> {
     unimplemented!()
 }
 
 pub fn live_patch(policy: &str) -> Result<()> {
-    let result = parse_sepolicy(policy.trim())?;
+    let result = parse_sepolicy(policy.trim());
     for statement in result {
         println!("{statement:?}");
         apply_one_rule(&statement)?;

userspace/ksud/src/utils.rs

@@ -1,11 +1,15 @@
 use anyhow::{bail, Context, Error, Ok, Result};
 use std::{
-    fs::{create_dir_all, set_permissions, write, File, Permissions},
+    fs::{create_dir_all, write, File},
     io::ErrorKind::AlreadyExists,
-    os::unix::prelude::PermissionsExt,
     path::Path,
 };
 
+#[allow(unused_imports)]
+use std::fs::{set_permissions, Permissions};
+#[cfg(unix)]
+use std::os::unix::prelude::PermissionsExt;
+
 pub fn ensure_clean_dir(dir: &str) -> Result<()> {
     let path = Path::new(dir);
     log::debug!("ensure_clean_dir: {}", path.display());
@@ -23,9 +27,8 @@ pub fn ensure_file_exists<T: AsRef<Path>>(file: T) -> Result<()> {
             if err.kind() == AlreadyExists && file.as_ref().is_file() {
                 Ok(())
             } else {
-                Err(Error::from(err)).with_context(|| {
+                Err(Error::from(err))
-                    format!("{} is not a regular file", file.as_ref().to_str().unwrap())
+                    .with_context(|| format!("{} is not a regular file", file.as_ref().display()))
-                })
             }
         }
     }
@@ -36,10 +39,7 @@ pub fn ensure_dir_exists<T: AsRef<Path>>(dir: T) -> Result<()> {
     if dir.as_ref().is_dir() {
         result
     } else if result.is_ok() {
-        bail!(
+        bail!("{} is not a regular directory", dir.as_ref().display())
-            "{} is not a regular directory",
-            dir.as_ref().to_str().unwrap()
-        )
     } else {
         result
     }
@@ -58,16 +58,17 @@ pub fn ensure_binary<T: AsRef<Path>>(path: T, contents: &[u8]) -> Result<()> {
     })?)?;
 
     write(&path, contents)?;
+    #[cfg(unix)]
     set_permissions(&path, Permissions::from_mode(0o755))?;
     Ok(())
 }
 
-#[cfg(target_os = "android")]
+#[cfg(unix)]
 pub fn getprop(prop: &str) -> Option<String> {
     android_properties::getprop(prop).value()
 }
 
-#[cfg(not(target_os = "android"))]
+#[cfg(not(unix))]
 pub fn getprop(_prop: &str) -> Option<String> {
     unimplemented!()
 }