From 3599dcedfb4bb3f7bb6e674f5a585b01c35aa3cf Mon Sep 17 00:00:00 2001 From: topjohnwu Date: Tue, 26 Aug 2025 23:37:16 -0700 Subject: [PATCH] Make argh directly parse into Utf8CString --- native/src/base/cstr.rs | 15 +++- native/src/base/misc.rs | 14 ++-- native/src/boot/cli.rs | 137 +++++++++++++++--------------------- native/src/boot/compress.rs | 15 +--- native/src/boot/cpio.rs | 2 +- native/src/boot/patch.rs | 4 +- native/src/sepolicy/cli.rs | 24 +++---- 7 files changed, 95 insertions(+), 116 deletions(-) diff --git a/native/src/base/cstr.rs b/native/src/base/cstr.rs index 06ac6fb2e..e4cc42919 100644 --- a/native/src/base/cstr.rs +++ b/native/src/base/cstr.rs @@ -7,7 +7,7 @@ use std::fmt::{Debug, Display, Formatter, Write}; use std::ops::Deref; use std::os::unix::ffi::OsStrExt; use std::path::{Path, PathBuf}; -use std::str::Utf8Error; +use std::str::{FromStr, Utf8Error}; use std::{fmt, mem, slice, str}; use thiserror::Error; @@ -200,7 +200,18 @@ impl From for Utf8CString { impl From<&str> for Utf8CString { fn from(value: &str) -> Self { - value.to_string().into() + let mut s = String::with_capacity(value.len() + 1); + s.push_str(value); + s.nul_terminate(); + Utf8CString(s) + } +} + +impl FromStr for Utf8CString { + type Err = String; + + fn from_str(s: &str) -> Result { + Ok(s.into()) } } diff --git a/native/src/base/misc.rs b/native/src/base/misc.rs index c270cf7b5..419da3df2 100644 --- a/native/src/base/misc.rs +++ b/native/src/base/misc.rs @@ -1,4 +1,4 @@ -use crate::{Utf8CStr, cstr, ffi}; +use crate::{Utf8CStr, Utf8CString, cstr, ffi}; use argh::{EarlyExit, MissingRequirements}; use libc::c_char; use std::{ @@ -105,9 +105,9 @@ impl EarlyExitExt for Result { pub struct PositionalArgParser<'a>(pub slice::Iter<'a, &'a str>); impl PositionalArgParser<'_> { - pub fn required(&mut self, field_name: &'static str) -> Result { + pub fn required(&mut self, field_name: &'static str) -> Result { if let Some(next) = self.0.next() { - Ok(next.to_string()) + Ok((*next).into()) } else { let mut missing = MissingRequirements::default(); missing.missing_positional_arg(field_name); @@ -116,17 +116,17 @@ impl PositionalArgParser<'_> { } } - pub fn optional(&mut self) -> Option { - self.0.next().map(ToString::to_string) + pub fn optional(&mut self) -> Option { + self.0.next().map(|s| (*s).into()) } - pub fn last_required(&mut self, field_name: &'static str) -> Result { + pub fn last_required(&mut self, field_name: &'static str) -> Result { let r = self.required(field_name)?; self.ensure_end()?; Ok(r) } - pub fn last_optional(&mut self) -> Result, EarlyExit> { + pub fn last_optional(&mut self) -> Result, EarlyExit> { let r = self.optional(); if r.is_none() { return Ok(r); diff --git a/native/src/boot/cli.rs b/native/src/boot/cli.rs index cfb4fc2f8..b66d65832 100644 --- a/native/src/boot/cli.rs +++ b/native/src/boot/cli.rs @@ -8,7 +8,7 @@ use crate::sign::{sha1_hash, sign_boot_image}; use argh::{CommandInfo, EarlyExit, FromArgs, SubCommand}; use base::{ CmdArgs, EarlyExitExt, LoggedResult, MappedFile, PositionalArgParser, ResultExt, Utf8CStr, - WriteExt, cmdline_logging, cstr, libc, libc::umask, log_err, + Utf8CString, WriteExt, cmdline_logging, cstr, libc, libc::umask, log_err, }; use std::ffi::c_char; use std::io::{Seek, SeekFrom, Write}; @@ -46,7 +46,7 @@ struct Unpack { #[argh(switch, short = 'h')] dump_header: bool, #[argh(positional)] - img: String, + img: Utf8CString, } #[derive(FromArgs)] @@ -55,33 +55,33 @@ struct Repack { #[argh(switch, short = 'n')] no_compress: bool, #[argh(positional)] - img: String, - #[argh(positional, default = r#""new-boot.img".to_string()"#)] - out: String, + img: Utf8CString, + #[argh(positional, default = r#"Utf8CString::from("new-boot.img")"#)] + out: Utf8CString, } #[derive(FromArgs)] #[argh(subcommand, name = "verify")] struct Verify { #[argh(positional)] - img: String, + img: Utf8CString, #[argh(positional)] - cert: Option, + cert: Option, } #[derive(FromArgs)] #[argh(subcommand, name = "sign")] struct Sign { #[argh(positional)] - img: String, + img: Utf8CString, #[argh(positional)] - args: Vec, + args: Vec, } struct Extract { - payload: String, - partition: Option, - outfile: Option, + payload: Utf8CString, + partition: Option, + outfile: Option, } impl FromArgs for Extract { @@ -106,18 +106,18 @@ impl SubCommand for Extract { #[argh(subcommand, name = "hexpatch")] struct HexPatch { #[argh(positional)] - file: String, + file: Utf8CString, #[argh(positional)] - src: String, + src: Utf8CString, #[argh(positional)] - dest: String, + dest: Utf8CString, } #[derive(FromArgs)] #[argh(subcommand, name = "cpio")] struct Cpio { #[argh(positional)] - file: String, + file: Utf8CString, #[argh(positional)] cmds: Vec, } @@ -126,7 +126,7 @@ struct Cpio { #[argh(subcommand, name = "dtb")] struct Dtb { #[argh(positional)] - file: String, + file: Utf8CString, #[argh(subcommand)] action: DtbAction, } @@ -137,14 +137,14 @@ struct Split { #[argh(switch, short = 'n')] no_decompress: bool, #[argh(positional)] - file: String, + file: Utf8CString, } #[derive(FromArgs)] #[argh(subcommand, name = "sha1")] struct Sha1 { #[argh(positional)] - file: String, + file: Utf8CString, } #[derive(FromArgs)] @@ -153,8 +153,8 @@ struct Cleanup {} struct Compress { format: FileFormat, - file: String, - out: Option, + file: Utf8CString, + out: Option, } impl FromArgs for Compress { @@ -185,8 +185,8 @@ impl SubCommand for Compress { } struct Decompress { - file: String, - out: Option, + file: Utf8CString, + out: Option, } impl FromArgs for Decompress { @@ -357,7 +357,7 @@ fn boot_main(cmds: CmdArgs) -> LoggedResult { cmds[1] = &cmds[1][2..]; } - let mut cli = if cmds[1].starts_with("compress=") { + let cli = if cmds[1].starts_with("compress=") { // Skip the main parser, directly parse the subcommand Compress::from_args(&cmds[..2], &cmds[2..]).map(|compress| Cli { action: Action::Compress(compress), @@ -375,40 +375,29 @@ fn boot_main(cmds: CmdArgs) -> LoggedResult { Action::Unpack(Unpack { no_decompress, dump_header, - ref mut img, + img, }) => { - return Ok(unpack( - Utf8CStr::from_string(img), - no_decompress, - dump_header, - )); + return Ok(unpack(&img, no_decompress, dump_header)); } Action::Repack(Repack { no_compress, - mut img, - mut out, + img, + out, }) => { - repack( - Utf8CStr::from_string(&mut img), - Utf8CStr::from_string(&mut out), - no_compress, - ); + repack(&img, &out, no_compress); } - Action::Verify(Verify { mut img, mut cert }) => { - if !verify_cmd( - Utf8CStr::from_string(&mut img), - cert.as_mut().map(Utf8CStr::from_string), - ) { + Action::Verify(Verify { img, cert }) => { + if !verify_cmd(&img, cert.as_deref()) { return log_err!(); } } - Action::Sign(Sign { mut img, mut args }) => { - let mut iter = args.iter_mut(); + Action::Sign(Sign { img, args }) => { + let mut iter = args.iter(); sign_cmd( - Utf8CStr::from_string(&mut img), - iter.next().map(Utf8CStr::from_string), - iter.next().map(Utf8CStr::from_string), - iter.next().map(Utf8CStr::from_string), + &img, + iter.next().map(AsRef::as_ref), + iter.next().map(AsRef::as_ref), + iter.next().map(AsRef::as_ref), )?; } Action::Extract(Extract { @@ -416,42 +405,34 @@ fn boot_main(cmds: CmdArgs) -> LoggedResult { partition, outfile, }) => { - extract_boot_from_payload(&payload, partition.as_deref(), outfile.as_deref()) - .log_with_msg(|w| w.write_str("Failed to extract from payload"))?; + extract_boot_from_payload( + &payload, + partition.as_ref().map(AsRef::as_ref), + outfile.as_ref().map(AsRef::as_ref), + ) + .log_with_msg(|w| w.write_str("Failed to extract from payload"))?; } - Action::HexPatch(HexPatch { - mut file, - mut src, - mut dest, - }) => { - if !hexpatch( - &mut file, - Utf8CStr::from_string(&mut src), - Utf8CStr::from_string(&mut dest), - ) { + Action::HexPatch(HexPatch { file, src, dest }) => { + if !hexpatch(&file, &src, &dest) { log_err!("Failed to patch")?; } } - Action::Cpio(Cpio { mut file, mut cmds }) => { - cpio_commands(Utf8CStr::from_string(&mut file), &mut cmds) - .log_with_msg(|w| w.write_str("Failed to process cpio"))?; + Action::Cpio(Cpio { file, cmds }) => { + cpio_commands(&file, &cmds).log_with_msg(|w| w.write_str("Failed to process cpio"))?; } - Action::Dtb(Dtb { mut file, action }) => { - return dtb_commands(Utf8CStr::from_string(&mut file), &action) + Action::Dtb(Dtb { file, action }) => { + return dtb_commands(&file, &action) .map(|b| if b { 0 } else { 1 }) .log_with_msg(|w| w.write_str("Failed to process dtb")); } Action::Split(Split { no_decompress, - mut file, + file, }) => { - return Ok(split_image_dtb( - Utf8CStr::from_string(&mut file), - no_decompress, - )); + return Ok(split_image_dtb(&file, no_decompress)); } - Action::Sha1(Sha1 { mut file }) => { - let file = MappedFile::open(Utf8CStr::from_string(&mut file))?; + Action::Sha1(Sha1 { file }) => { + let file = MappedFile::open(&file)?; let mut sha1 = [0u8; 20]; sha1_hash(file.as_ref(), &mut sha1); for byte in &sha1 { @@ -463,15 +444,11 @@ fn boot_main(cmds: CmdArgs) -> LoggedResult { eprintln!("Cleaning up..."); cleanup(); } - Action::Decompress(Decompress { mut file, mut out }) => { - decompress_cmd(&mut file, out.as_mut())?; + Action::Decompress(Decompress { file, out }) => { + decompress_cmd(&file, out.as_deref())?; } - Action::Compress(Compress { - format, - mut file, - mut out, - }) => { - compress_cmd(format, &mut file, out.as_mut())?; + Action::Compress(Compress { format, file, out }) => { + compress_cmd(format, &file, out.as_deref())?; } } Ok(0) diff --git a/native/src/boot/compress.rs b/native/src/boot/compress.rs index 8e189b98a..f5a1a57ed 100644 --- a/native/src/boot/compress.rs +++ b/native/src/boot/compress.rs @@ -284,13 +284,7 @@ pub fn decompress_bytes(format: FileFormat, in_bytes: &[u8], out_fd: RawFd) { // Command-line entry points -pub(crate) fn decompress_cmd( - infile: &mut String, - outfile: Option<&mut String>, -) -> LoggedResult<()> { - let infile = Utf8CStr::from_string(infile); - let outfile = outfile.map(Utf8CStr::from_string); - +pub(crate) fn decompress_cmd(infile: &Utf8CStr, outfile: Option<&Utf8CStr>) -> LoggedResult<()> { let in_std = infile == "-"; let mut rm_in = false; @@ -353,12 +347,9 @@ pub(crate) fn decompress_cmd( pub(crate) fn compress_cmd( method: FileFormat, - infile: &mut String, - outfile: Option<&mut String>, + infile: &Utf8CStr, + outfile: Option<&Utf8CStr>, ) -> LoggedResult<()> { - let infile = Utf8CStr::from_string(infile); - let outfile = outfile.map(Utf8CStr::from_string); - let in_std = infile == "-"; let mut rm_in = false; diff --git a/native/src/boot/cpio.rs b/native/src/boot/cpio.rs index 6d2266996..aab1bb0e5 100644 --- a/native/src/boot/cpio.rs +++ b/native/src/boot/cpio.rs @@ -754,7 +754,7 @@ impl Display for CpioEntry { } } -pub(crate) fn cpio_commands(file: &Utf8CStr, cmds: &mut Vec) -> LoggedResult<()> { +pub(crate) fn cpio_commands(file: &Utf8CStr, cmds: &Vec) -> LoggedResult<()> { let mut cpio = if file.exists() { Cpio::load_from_file(file)? } else { diff --git a/native/src/boot/patch.rs b/native/src/boot/patch.rs index 5535217f0..12d51a43e 100644 --- a/native/src/boot/patch.rs +++ b/native/src/boot/patch.rs @@ -102,9 +102,9 @@ fn hex2byte(hex: &[u8]) -> Vec { v } -pub fn hexpatch(file: &mut String, from: &Utf8CStr, to: &Utf8CStr) -> bool { +pub fn hexpatch(file: &Utf8CStr, from: &Utf8CStr, to: &Utf8CStr) -> bool { let res: LoggedResult = try { - let mut map = MappedFile::open_rw(Utf8CStr::from_string(file))?; + let mut map = MappedFile::open_rw(file)?; let pattern = hex2byte(from.as_bytes()); let patch = hex2byte(to.as_bytes()); diff --git a/native/src/sepolicy/cli.rs b/native/src/sepolicy/cli.rs index 65da5ef06..872f879a8 100644 --- a/native/src/sepolicy/cli.rs +++ b/native/src/sepolicy/cli.rs @@ -2,8 +2,8 @@ use crate::ffi::SePolicy; use crate::statement::format_statement_help; use argh::FromArgs; use base::{ - CmdArgs, EarlyExitExt, FmtAdaptor, LoggedResult, Utf8CStr, cmdline_logging, cstr, libc::umask, - log_err, + CmdArgs, EarlyExitExt, FmtAdaptor, LoggedResult, Utf8CString, cmdline_logging, cstr, + libc::umask, log_err, }; use std::ffi::c_char; use std::io::stderr; @@ -26,13 +26,13 @@ struct Cli { print_rules: bool, #[argh(option)] - load: Option, + load: Option, #[argh(option)] - save: Option, + save: Option, #[argh(option)] - apply: Vec, + apply: Vec, #[argh(positional)] polices: Vec, @@ -85,10 +85,10 @@ pub unsafe extern "C" fn main( print_usage(cmds.first().unwrap_or(&"magiskpolicy")); return 1; } - let mut cli = Cli::from_args(&[cmds[0]], &cmds[1..]).on_early_exit(|| print_usage(cmds[0])); + let cli = Cli::from_args(&[cmds[0]], &cmds[1..]).on_early_exit(|| print_usage(cmds[0])); - let mut sepol = match (&mut cli.load, cli.load_split, cli.compile_split) { - (Some(file), false, false) => SePolicy::from_file(Utf8CStr::from_string(file)), + let mut sepol = match (cli.load, cli.load_split, cli.compile_split) { + (Some(file), false, false) => SePolicy::from_file(&file), (None, true, false) => SePolicy::from_split(), (None, false, true) => SePolicy::compile_split(), (None, false, false) => SePolicy::from_file(cstr!("/sys/fs/selinux/policy")), @@ -115,8 +115,8 @@ pub unsafe extern "C" fn main( sepol.magisk_rules(); } - for file in &mut cli.apply { - sepol.load_rule_file(Utf8CStr::from_string(file)); + for file in cli.apply { + sepol.load_rule_file(&file); } for statement in &cli.polices { @@ -127,8 +127,8 @@ pub unsafe extern "C" fn main( log_err!("Cannot apply policy")?; } - if let Some(file) = &mut cli.save { - if !sepol.to_file(Utf8CStr::from_string(file)) { + if let Some(file) = cli.save { + if !sepol.to_file(&file) { log_err!("Cannot dump policy to {}", file)?; } }