fix(app): fix package config change crash and harden config handling

apd/src/package.rs

@@ -1,6 +1,7 @@
 use std::{
-    fs::File,
+    fs::{File, OpenOptions},
     io::{self, BufRead},
+    os::unix::io::AsRawFd,
     path::Path,
     thread,
     time::Duration,
@@ -19,14 +20,8 @@ pub struct PackageConfig {
     pub sctx: String,
 }
 
-/// Outcome of reading package_config, distinguishing a legitimately empty
-/// config (header-only, i.e. user revoked everything) from a torn/unreadable
-/// one (missing, 0-byte, truncated) that must NOT trigger revokes.
 pub struct PackageConfigRead {
     pub configs: Vec<PackageConfig>,
-    /// true if the file was opened and every row parsed cleanly.
-    /// Header-only parses as valid-but-empty. 0-byte / missing / dirty
-    /// files are invalid after retries.
     pub valid: bool,
 }
 
@@ -77,7 +72,6 @@ pub fn read_ap_package_config_validated() -> PackageConfigRead {
             .from_reader(file);
         let mut package_configs = Vec::new();
         let mut success = true;
-        let mut saw_any_row = false;
 
         for record in reader.records() {
             let record = match record {
@@ -91,7 +85,6 @@ pub fn read_ap_package_config_validated() -> PackageConfigRead {
             if is_blank_record(&record) {
                 continue;
             }
-            saw_any_row = true;
             if is_header_record(&record) {
                 continue;
             }
@@ -112,13 +105,6 @@ pub fn read_ap_package_config_validated() -> PackageConfigRead {
             }
         }
 
-        // A 0-byte / blank-only file has no header and no rows: almost
-        // certainly a torn read, not a legit "revoke everything".
-        if success && !saw_any_row {
-            warn!("package_config has no parsable rows (empty?), treating as torn read");
-            success = false;
-        }
-
         if success {
             return PackageConfigRead {
                 configs: package_configs,
@@ -182,6 +168,12 @@ pub fn write_ap_package_config(package_configs: &[PackageConfig]) -> io::Result<
             continue;
         }
 
+        if let Err(e) = writer.get_ref().sync_all() {
+            warn!("Error syncing temp file: {}", e);
+            thread::sleep(Duration::from_secs(1));
+            continue;
+        }
+
         if let Err(e) = std::fs::rename(temp_path, "/data/adb/ap/package_config") {
             warn!("Error renaming temp file: {}", e);
             thread::sleep(Duration::from_secs(1));
@@ -192,6 +184,53 @@ pub fn write_ap_package_config(package_configs: &[PackageConfig]) -> io::Result<
     Err(io::Error::other("Failed after max retries"))
 }
 
+struct PackageConfigLock(File);
+
+impl PackageConfigLock {
+    fn acquire() -> Option<PackageConfigLock> {
+        let file = match OpenOptions::new()
+            .read(true)
+            .write(true)
+            .create(true)
+            .open("/data/adb/ap/package_config.lock")
+        {
+            Ok(file) => file,
+            Err(e) => {
+                warn!("Failed to open config lock, proceeding unlocked: {}", e);
+                return None;
+            }
+        };
+        let fl = libc::flock {
+            l_type: libc::F_WRLCK as i16,
+            l_whence: libc::SEEK_SET as i16,
+            l_start: 0,
+            l_len: 0,
+            l_pid: 0,
+        };
+        if unsafe { libc::fcntl(file.as_raw_fd(), libc::F_SETLKW, &fl) } != 0 {
+            warn!(
+                "Failed to acquire config lock, proceeding unlocked: {}",
+                io::Error::last_os_error()
+            );
+            return None;
+        }
+        Some(PackageConfigLock(file))
+    }
+}
+
+impl Drop for PackageConfigLock {
+    fn drop(&mut self) {
+        let fl = libc::flock {
+            l_type: libc::F_UNLCK as i16,
+            l_whence: libc::SEEK_SET as i16,
+            l_start: 0,
+            l_len: 0,
+            l_pid: 0,
+        };
+        unsafe { libc::fcntl(self.0.as_raw_fd(), libc::F_SETLK, &fl) };
+    }
+}
+
 fn read_lines<P>(filename: P) -> io::Result<io::Lines<io::BufReader<File>>>
 where
     P: AsRef<Path>,
@@ -212,15 +251,6 @@ pub fn synchronize_package_uid() -> io::Result<()> {
                 #[allow(clippy::lines_filter_map_ok)]
                 let lines: Vec<_> = lines.filter_map(|line| line.ok()).collect();
 
-                // A torn package_config read must fail-open: syncing (and
-                // persisting) against an empty snapshot would drop grants.
-                let read = read_ap_package_config_validated();
-                if !read.valid {
-                    warn!("[synchronize_package_uid] package_config unreadable, skipping sync");
-                    return Err(io::Error::other("package_config is unreadable"));
-                }
-                let mut package_configs = read.configs;
-
                 let system_packages: Vec<String> = lines
                     .iter()
                     .filter_map(|line| line.split_whitespace().next())
@@ -236,6 +266,17 @@ pub fn synchronize_package_uid() -> io::Result<()> {
                     return Err(io::Error::other("packages.list is empty"));
                 }
 
+                let _lock = PackageConfigLock::acquire();
+
+                // A torn package_config read must fail-open: syncing (and
+                // persisting) against an empty snapshot would drop grants.
+                let read = read_ap_package_config_validated();
+                if !read.valid {
+                    warn!("[synchronize_package_uid] package_config unreadable, skipping sync");
+                    return Err(io::Error::other("package_config is unreadable"));
+                }
+                let mut package_configs = read.configs;
+
                 let original_len = package_configs.len();
                 package_configs.retain(|config| system_packages.contains(&config.pkg));
                 let removed_count = original_len - package_configs.len();

app/src/main/java/me/bmax/apatch/util/PkgConfig.kt

@@ -17,6 +17,7 @@ object PkgConfig {
     private const val TAG = "PkgConfig"
 
     private const val CSV_HEADER = "pkg,exclude,allow,uid,to_uid,sctx"
+    private val CSV_COLUMNS = CSV_HEADER.split(',')
 
     @Immutable
     @Parcelize
@@ -50,6 +51,12 @@ object PkgConfig {
         }
     }
 
+    private fun isHeaderLine(line: String): Boolean {
+        val columns = line.trimStart().removePrefix("\uFEFF").split(',')
+        if (columns.size != CSV_COLUMNS.size) return false
+        return columns.indices.all { columns[it].trim() == CSV_COLUMNS[it] }
+    }
+
     fun readConfigs(): HashMap<Int, Config> {
         val configs = HashMap<Int, Config>()
         val file = File(APApplication.PACKAGE_CONFIG_FILE)
@@ -58,8 +65,7 @@ object PkgConfig {
                 Log.d(TAG, it)
                 // Skip the CSV header (and a possible UTF-8 BOM) quietly:
                 // it is not a malformed row.
-                val stripped = it.trimStart().removePrefix("\uFEFF")
-                if (stripped == CSV_HEADER || stripped.startsWith("pkg,")) return@forEach
+                if (isHeaderLine(it)) return@forEach
                 val p = Config.fromLine(it)
                 if (p == null) {
                     Log.w(TAG, "Skip malformed package_config line: $it")
@@ -110,18 +116,31 @@ object PkgConfig {
             "package_config.lock"
         )
         lockFile.parentFile?.mkdirs()
-        return RandomAccessFile(lockFile, "rw").use { raf ->
-            raf.channel.use { channel ->
-                channel.lock().use { block() }
+        val raf = try {
+            RandomAccessFile(lockFile, "rw")
+        } catch (e: Exception) {
+            Log.w(TAG, "Failed to open config lock, proceeding unlocked", e)
+            return block()
+        }
+        return raf.use { r ->
+            val lock = try {
+                r.channel.lock()
+            } catch (e: Exception) {
+                Log.w(TAG, "Failed to acquire config lock, proceeding unlocked", e)
+                return@use block()
             }
+            lock.use { block() }
         }
     }
 
     fun changeConfig(config: Config) {
         thread {
             synchronized(PkgConfig.javaClass) {
+                if (!Natives.su()) {
+                    Log.e(TAG, "su failed, abort change config")
+                    return@synchronized
+                }
                 withConfigLock {
-                    Natives.su()
                     val configs = readConfigs()
                     val uid = config.profile.uid
                     // Root App should not be excluded