ksud: Fix module may mount failed

userspace/ksud/src/event.rs

@@ -124,7 +124,7 @@ pub fn on_post_data_fs() -> Result<()> {
     }
 
     info!("mount {target_update_img} to {module_dir}");
-    mount::mount_ext4(target_update_img, module_dir)?;
+    mount::mount_ext4(target_update_img, module_dir, false)?;
 
     // load sepolicy.rule
     if crate::module::load_sepolicy_rule().is_err() {

userspace/ksud/src/module.rs

@@ -129,6 +129,8 @@ fn grow_image_size(img: &str, extra_size: u64) -> Result<()> {
         .with_context(|| format!("Failed to exec resize2fs {img}"))?;
     ensure!(result.success(), "Failed to resize2fs: {}", result);
 
+    check_image(img)?;
+
     Ok(())
 }
 
@@ -435,8 +437,14 @@ fn do_install_module(zip: String) -> Result<()> {
     let zip_uncompressed_size = get_zip_uncompressed_size(&zip)?;
     let grow_size = default_reserve_size + zip_uncompressed_size;
 
-    info!("zip uncompressed size: {}", humansize::format_size(zip_uncompressed_size, humansize::DECIMAL));
+    info!(
-    info!("grow size: {}", humansize::format_size(grow_size, humansize::DECIMAL));
+        "zip uncompressed size: {}",
+        humansize::format_size(zip_uncompressed_size, humansize::DECIMAL)
+    );
+    info!(
+        "grow size: {}",
+        humansize::format_size(grow_size, humansize::DECIMAL)
+    );
 
     println!("- Preparing image");
     println!(
@@ -497,33 +505,33 @@ fn do_install_module(zip: String) -> Result<()> {
     // mount the modules_update.img to mountpoint
     println!("- Mounting image");
 
-    mount::mount_ext4(tmp_module_img, module_update_tmp_dir)?;
+    {
+        // we need auto drop, so we use a block here
+        mount::mount_ext4(tmp_module_img, module_update_tmp_dir, true)?;
 
-    setsyscon(module_update_tmp_dir)?;
+        setsyscon(module_update_tmp_dir)?;
 
-    let module_dir = format!("{module_update_tmp_dir}/{module_id}");
+        let module_dir = format!("{module_update_tmp_dir}/{module_id}");
-    ensure_clean_dir(&module_dir)?;
+        ensure_clean_dir(&module_dir)?;
-    info!("module dir: {}", module_dir);
+        info!("module dir: {}", module_dir);
 
-    // unzip the image and move it to modules_update/<id> 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)?;
+        let mut archive = zip::ZipArchive::new(file)?;
-    archive.extract(&module_dir)?;
+        archive.extract(&module_dir)?;
 
-    // set selinux for module/system dir
+        // set selinux for module/system dir
-    let mut module_system_dir = PathBuf::from(module_dir);
+        let mut module_system_dir = PathBuf::from(module_dir);
-    module_system_dir.push("system");
+        module_system_dir.push("system");
-    let module_system_dir = module_system_dir.as_path();
+        let module_system_dir = module_system_dir.as_path();
-    if module_system_dir.exists() {
+        if module_system_dir.exists() {
-        let path = module_system_dir.to_str().unwrap();
+            let path = module_system_dir.to_str().unwrap();
-        restore_syscon(path)?;
+            restore_syscon(path)?;
+        }
+
+        exec_install_script(&zip)?;
     }
 
-    exec_install_script(&zip)?;
-
-    // umount the modules_update.img
-    mount::umount_dir(module_update_tmp_dir)?;
-
     // remove modules_update dir, ignore the error
     remove_dir_all(module_update_tmp_dir)?;
 
@@ -580,7 +588,7 @@ where
     ensure_clean_dir(update_dir)?;
 
     // mount the modules_update img
-    mount::mount_ext4(defs::MODULE_UPDATE_TMP_IMG, update_dir)?;
+    mount::mount_ext4(defs::MODULE_UPDATE_TMP_IMG, update_dir, true)?;
 
     // call the operation func
     let result = func(id, update_dir);

userspace/ksud/src/mount.rs

@@ -1,28 +1,103 @@
 use anyhow::Result;
 
 #[cfg(target_os = "android")]
-use anyhow::{ensure, Context, Ok};
+use anyhow::{Context, Ok};
 #[cfg(target_os = "android")]
 use retry::delay::NoDelay;
 #[cfg(target_os = "android")]
-use sys_mount::{unmount, FilesystemType, Mount, MountFlags, UnmountFlags};
+use sys_mount::{unmount, FilesystemType, Mount, MountFlags, Unmount, UnmountFlags};
 
 #[cfg(target_os = "android")]
-fn do_mount_image(src: &str, target: &str) -> Result<()> {
+struct AutoMountExt4 {
-    Mount::builder()
+    mnt: String,
-        .fstype(FilesystemType::from("ext4"))
+    mount: Option<Mount>,
-        .mount(src, target)
+}
-        .with_context(|| format!("Failed to do mount: {src} -> {target}"))?;
+
+#[cfg(target_os = "android")]
+#[allow(dead_code)]
+impl AutoMountExt4 {
+    pub fn try_new(src: &str, mnt: &str) -> Result<Self> {
+        let result = Mount::builder()
+            .fstype(FilesystemType::from("ext4"))
+            .flags(MountFlags::empty())
+            .mount(src, mnt)
+            .map(|mount| {
+                Ok(Self {
+                    mnt: mnt.to_string(),
+                    mount: Some(mount),
+                })
+            });
+        if let Err(e) = result {
+            println!("- Mount failed: {}, retry with system mount", e);
+            let result = std::process::Command::new("mount")
+                .arg("-t")
+                .arg("ext4")
+                .arg(src)
+                .arg(mnt)
+                .status();
+            if let Err(e) = result {
+                Err(anyhow::anyhow!(
+                    "mount partition: {src} -> {mnt} failed: {e}"
+                ))
+            } else {
+                Ok(Self {
+                    mnt: mnt.to_string(),
+                    mount: None,
+                })
+            }
+        } else {
+            result.unwrap()
+        }
+    }
+
+    pub fn umount(&self) -> Result<()> {
+        match self.mount {
+            Some(ref mount) => mount
+                .unmount(UnmountFlags::empty())
+                .map_err(|e| anyhow::anyhow!(e)),
+            None => {
+                let result = std::process::Command::new("umount").arg(&self.mnt).status();
+                if let Err(e) = result {
+                    Err(anyhow::anyhow!("umount: {} failed: {e}", self.mnt))
+                } else {
+                    Ok(())
+                }
+            }
+        }
+    }
+}
+
+#[cfg(target_os = "android")]
+impl Drop for AutoMountExt4 {
+    fn drop(&mut self) {
+        let _ = self.umount();
+    }
+}
+
+#[cfg(target_os = "android")]
+fn do_mount_image(src: &str, target: &str, autodrop: bool) -> Result<()> {
+    if autodrop {
+        Mount::builder()
+            .fstype(FilesystemType::from("ext4"))
+            .mount_autodrop(src, target, UnmountFlags::empty())
+            .with_context(|| format!("Failed to do mount: {src} -> {target}"))?;
+    } else {
+        Mount::builder()
+            .fstype(FilesystemType::from("ext4"))
+            .mount(src, target)
+            .with_context(|| format!("Failed to do mount: {src} -> {target}"))?;
+    }
     Ok(())
 }
 
 #[cfg(target_os = "android")]
-pub fn mount_ext4(src: &str, target: &str) -> Result<()> {
+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));
+    let result = retry::retry(NoDelay.take(3), || do_mount_image(src, target, autodrop));
-    ensure!(result.is_ok(), "Failed to mount {} -> {}", src, target);
+    result
-    Ok(())
+        .map_err(|e| anyhow::anyhow!("mount partition: {src} -> {target} failed: {e}"))
+        .map(|_| ())
 }
 
 #[cfg(target_os = "android")]
@@ -43,7 +118,7 @@ pub fn mount_overlay(lowerdir: &str, mnt: &str) -> Result<()> {
 }
 
 #[cfg(not(target_os = "android"))]
-pub fn mount_ext4(_src: &str, _target: &str) -> Result<()> {
+pub fn mount_ext4(_src: &str, _target: &str, autodrop: bool) -> Result<()> {
     unimplemented!()
 }