From d0fc0379b94fbe89e5f8561591d5b42cf370f770 Mon Sep 17 00:00:00 2001 From: va-resident Date: Fri, 18 Sep 2026 18:43:06 +0300 Subject: [PATCH] Fix mountinfo read-write detection across both option layers /proc/PID/mountinfo carries two option sets with different meaning: fields[5] are the per-mount (VFS) flags, the field after the "-" separator belongs to the superblock. A mount is writable only if both allow it. parse_mountinfo() merged both into one list and set is_read_write = "rw" in options, so a "rw" VFS mount over a read-only superblock was reported as writable. On stock Xiaomi-family builds that made every read-only mi_ext customisation overlay a HIGH "system partition is mounted as read-write". Require "rw" in both layers, and let "rw" count as a suspicious mount option only when the mount is actually writable; remount, noatime and nodiratime keep their current meaning in either layer. mount_options and options_list still carry both layers, so nothing downstream loses data. Measured on 50 bug reports carrying mountinfo - the 28 where the rule changes the output plus 22 controls, 14 brands, Android 10-16: HIGH 94 -> 0, MEDIUM 119 -> 25, the 22 controls identical, and 36233 mount entries parsed either way. Every removed alert is a read-only superblock under a "rw" VFS mount; no report gains an alert. A partition that really is writable still raises the HIGH, which the new test asserts explicitly. Fixes #936 Co-Authored-By: Claude Opus 5 (1M context) --- src/mvt/android/artifacts/mounts.py | 15 ++++++++-- tests/android/test_artifact_mounts.py | 43 +++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 2 deletions(-) create mode 100644 tests/android/test_artifact_mounts.py diff --git a/src/mvt/android/artifacts/mounts.py b/src/mvt/android/artifacts/mounts.py index 5a5537b7..0380a785 100644 --- a/src/mvt/android/artifacts/mounts.py +++ b/src/mvt/android/artifacts/mounts.py @@ -136,6 +136,10 @@ class Mounts(AndroidArtifact): mount_options = fields[5].split(",") super_options = fields[separator + 3].split(",") options = list(dict.fromkeys(mount_options + super_options)) + # fields[5] are the per-mount (VFS) flags, the field after the + # separator the superblock's; a mount is writable only if both + # allow it. The merged list made every read-only overlay a HIGH. + is_read_write = "rw" in mount_options and "rw" in super_options mount_point = fields[4].replace("\\040", " ") device = fields[separator + 2].replace("\\040", " ") filesystem_type = fields[separator + 1] @@ -156,7 +160,7 @@ class Mounts(AndroidArtifact): "options_list": options, "optional_fields": fields[6:separator], "is_system_partition": is_system, - "is_read_write": "rw" in options, + "is_read_write": is_read_write, "process_ids": [process_id], } ) @@ -191,7 +195,14 @@ class Mounts(AndroidArtifact): ) # Check for other suspicious mount options - suspicious_opts = [opt for opt in options if opt in SUSPICIOUS_OPTIONS] + # `rw` sitting in the merged option list is not evidence of + # writability (see is_read_write in parse_mountinfo); every other + # suspicious option is meaningful in either list. + suspicious_opts = [ + opt + for opt in options + if opt in SUSPICIOUS_OPTIONS and (opt != "rw" or mount["is_read_write"]) + ] if suspicious_opts and mount["is_system_partition"]: if ( "noatime" in mount["mount_options"] diff --git a/tests/android/test_artifact_mounts.py b/tests/android/test_artifact_mounts.py new file mode 100644 index 00000000..6b7d1faf --- /dev/null +++ b/tests/android/test_artifact_mounts.py @@ -0,0 +1,43 @@ +# Mobile Verification Toolkit (MVT) +# Copyright (c) 2021-2026 The MVT Authors. +# Use of this software is governed by the MVT License 1.1 that can be found at +# https://license.mvt.re/1.1/ +"""A mount is writable only if both mountinfo option layers allow it.""" + +from mvt.android.artifacts.mounts import Mounts +from mvt.common.alerts import AlertLevel + +# fields[5] is the per-mount (VFS) layer, the field after "-" the superblock. +RO_OVERLAY = ( + "304 166 0:130 / /product/usr rw,relatime shared:63 - overlay overlay " + "ro,seclabel,lowerdir=/mnt/vendor/ext/product/usr:/product/usr" +) +RW_SYSTEM = ( + "305 166 0:131 / /system rw,relatime shared:64 - ext4 /dev/block/dm-1 " + "rw,seclabel,errors=panic" +) + + +def _run(line): + mounts = Mounts() + mounts.results = Mounts.parse_mountinfo(line, 1) + mounts.check_indicators() + return mounts + + +class TestMountsReadWriteLayers: + def test_rw_vfs_over_ro_superblock_is_not_writable(self): + mounts = _run(RO_OVERLAY) + assert mounts.results[0]["is_read_write"] is False + assert mounts.alertstore.count(AlertLevel.HIGH) == 0 + assert mounts.alertstore.count(AlertLevel.MEDIUM) == 0 + + def test_rw_in_both_layers_still_alerts(self): + mounts = _run(RW_SYSTEM) + assert mounts.results[0]["is_read_write"] is True + assert mounts.alertstore.count(AlertLevel.HIGH) == 1 + + def test_both_option_layers_are_still_reported(self): + mounts = _run(RO_OVERLAY) + options = mounts.results[0]["options_list"] + assert "rw" in options and "ro" in options