From 1d56fe6538d726531d09f419b5f7428359e01667 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Tue, 18 May 2021 17:11:35 +0800 Subject: [PATCH 01/10] fs_mgr_vendor_overlay: Mount vendor overlay with noatime Change relatime to noatime as our filesystems and partitions are all read-only, so relatime doesn't really work in the first place. Bug: 188862155 Test: atest -v fs_mgr_vendor_overlay_test Test: Presubmit Change-Id: I869bba24c2d6d8a986bf0eec0f487ba7b935ab4b Merged-In: I869bba24c2d6d8a986bf0eec0f487ba7b935ab4b (cherry picked from commit 7cbed5a9cec9df1a5ce99f9b61743ffd7e8fe7b7) --- fs_mgr/fs_mgr_vendor_overlay.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs_mgr/fs_mgr_vendor_overlay.cpp b/fs_mgr/fs_mgr_vendor_overlay.cpp index 830f0dd01..1372511a7 100644 --- a/fs_mgr/fs_mgr_vendor_overlay.cpp +++ b/fs_mgr/fs_mgr_vendor_overlay.cpp @@ -92,7 +92,7 @@ bool fs_mgr_vendor_overlay_mount(const std::pair& moun } auto report = "__mount(source=overlay,target="s + vendor_mount_point + ",type=overlay," + options + ")="; - auto ret = mount("overlay", vendor_mount_point.c_str(), "overlay", MS_RDONLY | MS_RELATIME, + auto ret = mount("overlay", vendor_mount_point.c_str(), "overlay", MS_RDONLY | MS_NOATIME, options.c_str()); if (ret) { PERROR << report << ret; From aa061738bc26dffd3fbfe6245c09fb5293ff94b3 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Wed, 19 May 2021 16:02:46 +0800 Subject: [PATCH 02/10] first_stage_mount: Remove "overlay" hack from InitRequiredDevices() GetDmVerityDevices() should filter out overlayfs fstab entries in the first place, so InitRequiredDevices() don't need to filter out overlayfs pseudo device names. Bug: 188862155 Test: Boot to normal with overlayfs mount entries in first stage fstab Change-Id: I0ac8b7ac0f21daa0c191580d9349adf217854864 Merged-In: I0ac8b7ac0f21daa0c191580d9349adf217854864 (cherry picked from commit 87290f8e9bb11b919e049e3662819af81e2a35c9) --- init/first_stage_mount.cpp | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index a73383914..87e23abc1 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -331,12 +331,6 @@ bool FirstStageMount::InitRequiredDevices(std::set devices) { if (devices.empty()) { return true; } - // excluding overlays - for (auto iter = devices.begin(); iter != devices.end(); ) { - if (*iter=="overlay") iter = devices.erase(iter); - else iter++; - } - return block_dev_init_.InitDevices(std::move(devices)); } @@ -695,6 +689,10 @@ bool FirstStageMountVBootV1::GetDmVerityDevices(std::set* devices) // Includes the partition names of fstab records. // Notes that fstab_rec->blk_device has A/B suffix updated by fs_mgr when A/B is used. for (const auto& fstab_entry : fstab_) { + // Skip pseudo filesystems. + if (fstab_entry.fs_type == "overlay") { + continue; + } if (!fstab_entry.fs_mgr_flags.logical) { devices->emplace(basename(fstab_entry.blk_device.c_str())); } @@ -757,6 +755,10 @@ bool FirstStageMountVBootV2::GetDmVerityDevices(std::set* devices) if (fstab_entry.fs_mgr_flags.avb) { need_dm_verity_ = true; } + // Skip pseudo filesystems. + if (fstab_entry.fs_type == "overlay") { + continue; + } if (fstab_entry.fs_mgr_flags.logical) { // Don't try to find logical partitions via uevent regeneration. logical_partitions.emplace(basename(fstab_entry.blk_device.c_str())); From 94d1f3b4fa211ac7be787084c20265390ed1ac27 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Wed, 19 May 2021 16:25:34 +0800 Subject: [PATCH 03/10] fs_mgr_overlayfs: Polish fs_mgr_overlayfs_mount_fstab_entry() * Add logs. * Append "override_creds=off" overlayfs mount flag only if fs_mgr_overlayfs_valid() returns kOverrideCredsRequired. Pre-4.6 kernels or kernels without the override_creds patch don't need or don't recognize the override_creds mount flag. (Background: I832c8ca3fce0269bdef4ce988541adb7ba9662ed) * mkdir(mount_point) before mount() to ensure the mount point exists. This could happen if the mount point is in a tmpfs, such as /mnt. Bug: 188862155 Test: Boot to normal with overlayfs mount entries in first stage fstab Change-Id: I1a05696346610d7fd61de6d25c379520fd58ca9b Merged-In: I1a05696346610d7fd61de6d25c379520fd58ca9b (cherry picked from commit dcf1c1f46235637a6d1057c88e6d1fed88e1b90a) --- fs_mgr/fs_mgr_overlayfs.cpp | 28 +++++++++++++++++++++------- fs_mgr/include/fs_mgr_overlayfs.h | 2 +- init/first_stage_mount.cpp | 14 +++++++------- 3 files changed, 29 insertions(+), 15 deletions(-) diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp index 9a94d7912..925d03f28 100644 --- a/fs_mgr/fs_mgr_overlayfs.cpp +++ b/fs_mgr/fs_mgr_overlayfs.cpp @@ -92,7 +92,7 @@ bool fs_mgr_overlayfs_mount_all(Fstab*) { return false; } -bool fs_mgr_overlayfs_mount_fstab_entry(const std::string&, const std::string&) { +bool fs_mgr_overlayfs_mount_fstab_entry(const android::fs_mgr::FstabEntry&) { return false; } @@ -1299,16 +1299,30 @@ static void TryMountScratch() { } } -bool fs_mgr_overlayfs_mount_fstab_entry(const std::string& lowers, - const std::string& mount_point) { +bool fs_mgr_overlayfs_mount_fstab_entry(const android::fs_mgr::FstabEntry& entry) { if (fs_mgr_overlayfs_invalid()) return false; - std::string aux = "lowerdir=" + lowers + ",override_creds=off"; - auto rc = mount("overlay", mount_point.c_str(), "overlay", MS_RDONLY | MS_NOATIME, aux.c_str()); + // Create the mount point in case it doesn't exist. + mkdir(entry.mount_point.c_str(), 0755); - if (rc == 0) return true; + auto options = kLowerdirOption + entry.lowerdir; + if (fs_mgr_overlayfs_valid() == OverlayfsValidResult::kOverrideCredsRequired) { + options += ",override_creds=off"; + } - return false; + // Use .blk_device as the mount() source for debugging purposes. + // Overlayfs is pseudo filesystem, so the source device is a symbolic value and isn't used to + // back the filesystem. /proc/mounts would show the source as the device name of the mount. + auto report = "__mount(source=" + entry.blk_device + ",target=" + entry.mount_point + + ",type=overlay," + options + ")="; + auto ret = mount(entry.blk_device.c_str(), entry.mount_point.c_str(), "overlay", + MS_RDONLY | MS_NOATIME, options.c_str()); + if (ret) { + PERROR << report << ret; + return false; + } + LINFO << report << ret; + return true; } bool fs_mgr_overlayfs_mount_all(Fstab* fstab) { diff --git a/fs_mgr/include/fs_mgr_overlayfs.h b/fs_mgr/include/fs_mgr_overlayfs.h index ac95ef519..22d12e753 100644 --- a/fs_mgr/include/fs_mgr_overlayfs.h +++ b/fs_mgr/include/fs_mgr_overlayfs.h @@ -27,7 +27,7 @@ android::fs_mgr::Fstab fs_mgr_overlayfs_candidate_list(const android::fs_mgr::Fstab& fstab); bool fs_mgr_overlayfs_mount_all(android::fs_mgr::Fstab* fstab); -bool fs_mgr_overlayfs_mount_fstab_entry (const std::string& lowers, const std::string& mount_point); +bool fs_mgr_overlayfs_mount_fstab_entry(const android::fs_mgr::FstabEntry& entry); std::vector fs_mgr_overlayfs_required_devices(android::fs_mgr::Fstab* fstab); bool fs_mgr_overlayfs_setup(const char* backing = nullptr, const char* mount_point = nullptr, bool* change = nullptr, bool force = true); diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index 87e23abc1..616d28540 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -542,6 +542,7 @@ bool FirstStageMount::MountPartitions() { continue; } + // Handle overlayfs entries later. if (current->fs_type == "overlay") { ++current; continue; @@ -571,6 +572,12 @@ bool FirstStageMount::MountPartitions() { current = end; } + for (const auto& entry : fstab_) { + if (entry.fs_type == "overlay") { + fs_mgr_overlayfs_mount_fstab_entry(entry); + } + } + // If we don't see /system or / in the fstab, then we need to create an root entry for // overlayfs. if (!GetEntryForMountPoint(&fstab_, "/system") && !GetEntryForMountPoint(&fstab_, "/")) { @@ -596,13 +603,6 @@ bool FirstStageMount::MountPartitions() { }; MapScratchPartitionIfNeeded(&fstab_, init_devices); - for (auto current = fstab_.begin(); current != fstab_.end(); ) { - if (current->fs_type == "overlay") { - fs_mgr_overlayfs_mount_fstab_entry(current->lowerdir, current->mount_point); - } - ++current; - } - fs_mgr_overlayfs_mount_all(&fstab_); return true; From a38c89a2f07a4645a8d935ae6686a12f65a5d530 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Sat, 22 May 2021 22:14:53 +0800 Subject: [PATCH 04/10] fs_mgr_overlayfs_mount_fstab_entry(): Rename source device name adb-remount-test.sh would fail if a overlayfs named "overlay" is mounted in first_stage_init via fs_mgr_overlayfs_mount_fstab_entry(): [ FAILED ] overlay takeover unexpected at this phase Prepend the source device name with "overlay-", so that the overlayfs mounted by fs_mgr_overlayfs_mount_fstab_entry() mustn't be named "overlay". Bug: 188862155 Test: adb-remount-test.sh Change-Id: Ia0b3f1c92ed5650c3e584fba23758344a4733657 Merged-In: Ia0b3f1c92ed5650c3e584fba23758344a4733657 (cherry picked from commit 7b90e6936c66b67a6761f7ad086a68bbcaf23c05) --- fs_mgr/fs_mgr_overlayfs.cpp | 16 +++++++++------- 1 file changed, 9 insertions(+), 7 deletions(-) diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp index 925d03f28..9fb658e77 100644 --- a/fs_mgr/fs_mgr_overlayfs.cpp +++ b/fs_mgr/fs_mgr_overlayfs.cpp @@ -1310,13 +1310,15 @@ bool fs_mgr_overlayfs_mount_fstab_entry(const android::fs_mgr::FstabEntry& entry options += ",override_creds=off"; } - // Use .blk_device as the mount() source for debugging purposes. - // Overlayfs is pseudo filesystem, so the source device is a symbolic value and isn't used to - // back the filesystem. /proc/mounts would show the source as the device name of the mount. - auto report = "__mount(source=" + entry.blk_device + ",target=" + entry.mount_point + - ",type=overlay," + options + ")="; - auto ret = mount(entry.blk_device.c_str(), entry.mount_point.c_str(), "overlay", - MS_RDONLY | MS_NOATIME, options.c_str()); + // Use "overlay-" + entry.blk_device as the mount() source, so that adb-remout-test don't + // confuse this with adb remount overlay, whose device name is "overlay". + // Overlayfs is a pseudo filesystem, so the source device is a symbolic value and isn't used to + // back the filesystem. However the device name would be shown in /proc/mounts. + auto source = "overlay-" + entry.blk_device; + auto report = "__mount(source=" + source + ",target=" + entry.mount_point + ",type=overlay," + + options + ")="; + auto ret = mount(source.c_str(), entry.mount_point.c_str(), "overlay", MS_RDONLY | MS_NOATIME, + options.c_str()); if (ret) { PERROR << report << ret; return false; From 8570398065235ed519c3de58769519dfc4a27428 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Sat, 22 May 2021 21:54:00 +0800 Subject: [PATCH 05/10] adb-remount-test: Strengthen skip_administrative_mounts Refactor skip_administrative_mounts so that it filters by device name, mount point and filesystem type. (Was filtering by device name and mount point.) We need this because pseudo filesystems such as tmpfs and overlayfs can have free-formed device name: blah /mnt/mnt_point overlay ro,lowerdir=... However the filesystem type (third field of /proc/mounts) must be reflecting the actual filesystem, so to robustly filter out administrative filesystems, we have to check the filesystem type field. Bug: 188862155 Test: adb-remount-test.sh Change-Id: I5058cbe8a36920f25b73a5d5833e9fc5a096e90f Merged-In: I5058cbe8a36920f25b73a5d5833e9fc5a096e90f (cherry picked from commit f931ad94465c24b8056618f85978af14c052079f) --- fs_mgr/tests/adb-remount-test.sh | 51 ++++++++++++++++++++++++-------- 1 file changed, 38 insertions(+), 13 deletions(-) diff --git a/fs_mgr/tests/adb-remount-test.sh b/fs_mgr/tests/adb-remount-test.sh index 242fa93fa..0c5fe3d65 100755 --- a/fs_mgr/tests/adb-remount-test.sh +++ b/fs_mgr/tests/adb-remount-test.sh @@ -735,23 +735,46 @@ check_ne() { fi } +[ "USAGE: join_with + +Joins strings with delimiter" ] +join_with() { + if [ "${#}" -lt 2 ]; then + echo + return + fi + local delimiter="${1}" + local result="${2}" + shift 2 + for element in "${@}"; do + result+="${delimiter}${element}" + done + echo "${result}" +} + [ "USAGE: skip_administrative_mounts [data] < /proc/mounts Filters out all administrative (eg: sysfs) mounts uninteresting to the test" ] skip_administrative_mounts() { + local exclude_filesystems=( + "overlay" "tmpfs" "none" "sysfs" "proc" "selinuxfs" "debugfs" "bpf" + "binfmt_misc" "cg2_bpf" "pstore" "tracefs" "adb" "mtp" "ptp" "devpts" + "ramdumpfs" "binder" "securityfs" "functionfs" "rootfs" + ) + local exclude_devices=( + "[/]sys[/]kernel[/]debug" "[/]data[/]media" "[/]dev[/]block[/]loop[0-9]*" + "${exclude_filesystems[@]}" + ) + local exclude_mount_points=( + "[/]cache" "[/]mnt[/]scratch" "[/]mnt[/]vendor[/]persist" "[/]persist" + "[/]metadata" + ) if [ "data" = "${1}" ]; then - grep -v " /data " - else - cat - - fi | - grep -v \ - -e "^\(overlay\|tmpfs\|none\|sysfs\|proc\|selinuxfs\|debugfs\|bpf\) " \ - -e "^\(binfmt_misc\|cg2_bpf\|pstore\|tracefs\|adb\|mtp\|ptp\|devpts\) " \ - -e "^\(ramdumpfs\|binder\|/sys/kernel/debug\|securityfs\) " \ - -e " functionfs " \ - -e "^\(/data/media\|/dev/block/loop[0-9]*\) " \ - -e "^rootfs / rootfs rw," \ - -e " /\(cache\|mnt/scratch\|mnt/vendor/persist\|persist\|metadata\) " + exclude_mount_points+=("[/]data") + fi + awk '$1 !~ /^('"$(join_with "|" "${exclude_devices[@]}")"')$/ && + $2 !~ /^('"$(join_with "|" "${exclude_mount_points[@]}")"')$/ && + $3 !~ /^('"$(join_with "|" "${exclude_filesystems[@]}")"')$/' } [ "USAGE: skip_unrelated_mounts < /proc/mounts @@ -907,9 +930,11 @@ ACTIVE_SLOT=`get_active_slot` # Acquire list of system partitions +# KISS (assume system partition mount point is "/") PARTITIONS=`adb_su cat /vendor/etc/fstab* Date: Wed, 19 May 2021 17:19:58 +0800 Subject: [PATCH 06/10] Make fs_mgr_overlayfs_mount_fstab_entry() available for user builds Rename fs_mgr_overlayfs_mount_fstab_entry() to fs_mgr_mount_overlayfs_fstab_entry() and move it out of fs_mgr_overlayfs.cpp to make it available for user builds. Add checks to unsure overlayfs mount point doesn't contain symbolic link or /../. Check the mount point with an allowlist if user build. The mount point should either be /vendor, /product ... or their submounts, or strict submounts of /mnt/vendor and /mnt/product. Bug: 188862155 Test: Boot test with overlayfs mount entries on user build Change-Id: I3b60dfa4b63cf2ae0754f53d1d08365aa7be1ee0 Merged-In: I3b60dfa4b63cf2ae0754f53d1d08365aa7be1ee0 (cherry picked from commit 23816e84ca8821f303d9a3e753d7c050881bacd5) --- fs_mgr/fs_mgr.cpp | 69 +++++++++++++++++++++++++++++++ fs_mgr/fs_mgr_overlayfs.cpp | 32 -------------- fs_mgr/include/fs_mgr.h | 5 +++ fs_mgr/include/fs_mgr_overlayfs.h | 1 - init/first_stage_mount.cpp | 2 +- 5 files changed, 75 insertions(+), 34 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index ea9d33350..08ead7a06 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -2265,3 +2265,72 @@ std::string fs_mgr_get_super_partition_name(int slot) { } return LP_METADATA_DEFAULT_PARTITION_NAME; } + +bool fs_mgr_mount_overlayfs_fstab_entry(const FstabEntry& entry) { + auto overlayfs_valid_result = fs_mgr_overlayfs_valid(); + if (overlayfs_valid_result == OverlayfsValidResult::kNotSupported) { + LERROR << __FUNCTION__ << "(): kernel does not support overlayfs"; + return false; + } + +#if ALLOW_ADBD_DISABLE_VERITY == 0 + // Allowlist the mount point if user build. + static const std::vector kAllowedPaths = { + "/odm", "/odm_dlkm", "/oem", "/product", "/system_ext", "/vendor", "/vendor_dlkm", + }; + static const std::vector kAllowedPrefixes = { + "/mnt/product/", + "/mnt/vendor/", + }; + if (std::none_of(kAllowedPaths.begin(), kAllowedPaths.end(), + [&entry](const auto& path) -> bool { + return entry.mount_point == path || + StartsWith(entry.mount_point, path + "/"); + }) && + std::none_of(kAllowedPrefixes.begin(), kAllowedPrefixes.end(), + [&entry](const auto& prefix) -> bool { + return entry.mount_point != prefix && + StartsWith(entry.mount_point, prefix); + })) { + LERROR << __FUNCTION__ + << "(): mount point is forbidden on user build: " << entry.mount_point; + return false; + } +#endif // ALLOW_ADBD_DISABLE_VERITY == 0 + + // Create the mount point in case it doesn't exist. + mkdir(entry.mount_point.c_str(), 0755); + + // Ensure that mount point exists and doesn't contain symbolic link or /../. + std::string mount_point; + if (!Realpath(entry.mount_point, &mount_point)) { + PERROR << __FUNCTION__ << "(): failed to realpath " << entry.mount_point; + return false; + } + if (entry.mount_point != mount_point) { + LERROR << __FUNCTION__ << "(): mount point must be a canonicalized path: realpath " + << entry.mount_point << " = " << mount_point; + return false; + } + + auto options = "lowerdir=" + entry.lowerdir; + if (overlayfs_valid_result == OverlayfsValidResult::kOverrideCredsRequired) { + options += ",override_creds=off"; + } + + // Use "overlay-" + entry.blk_device as the mount() source, so that adb-remout-test don't + // confuse this with adb remount overlay, whose device name is "overlay". + // Overlayfs is a pseudo filesystem, so the source device is a symbolic value and isn't used to + // back the filesystem. However the device name would be shown in /proc/mounts. + auto source = "overlay-" + entry.blk_device; + auto report = "__mount(source=" + source + ",target=" + entry.mount_point + ",type=overlay," + + options + ")="; + auto ret = mount(source.c_str(), entry.mount_point.c_str(), "overlay", MS_RDONLY | MS_NOATIME, + options.c_str()); + if (ret) { + PERROR << report << ret; + return false; + } + LINFO << report << ret; + return true; +} diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp index 9fb658e77..cb0938363 100644 --- a/fs_mgr/fs_mgr_overlayfs.cpp +++ b/fs_mgr/fs_mgr_overlayfs.cpp @@ -92,10 +92,6 @@ bool fs_mgr_overlayfs_mount_all(Fstab*) { return false; } -bool fs_mgr_overlayfs_mount_fstab_entry(const android::fs_mgr::FstabEntry&) { - return false; -} - std::vector fs_mgr_overlayfs_required_devices(Fstab*) { return {}; } @@ -1299,34 +1295,6 @@ static void TryMountScratch() { } } -bool fs_mgr_overlayfs_mount_fstab_entry(const android::fs_mgr::FstabEntry& entry) { - if (fs_mgr_overlayfs_invalid()) return false; - - // Create the mount point in case it doesn't exist. - mkdir(entry.mount_point.c_str(), 0755); - - auto options = kLowerdirOption + entry.lowerdir; - if (fs_mgr_overlayfs_valid() == OverlayfsValidResult::kOverrideCredsRequired) { - options += ",override_creds=off"; - } - - // Use "overlay-" + entry.blk_device as the mount() source, so that adb-remout-test don't - // confuse this with adb remount overlay, whose device name is "overlay". - // Overlayfs is a pseudo filesystem, so the source device is a symbolic value and isn't used to - // back the filesystem. However the device name would be shown in /proc/mounts. - auto source = "overlay-" + entry.blk_device; - auto report = "__mount(source=" + source + ",target=" + entry.mount_point + ",type=overlay," + - options + ")="; - auto ret = mount(source.c_str(), entry.mount_point.c_str(), "overlay", MS_RDONLY | MS_NOATIME, - options.c_str()); - if (ret) { - PERROR << report << ret; - return false; - } - LINFO << report << ret; - return true; -} - bool fs_mgr_overlayfs_mount_all(Fstab* fstab) { auto ret = false; if (fs_mgr_overlayfs_invalid()) return ret; diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index 22c02ccf8..b8ebd63a4 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -131,3 +131,8 @@ int fs_mgr_remount_userdata_into_checkpointing(android::fs_mgr::Fstab* fstab); // Finds the dm_bow device on which this block device is stacked, or returns // empty string std::string fs_mgr_find_bow_device(const std::string& block_device); + +// Like fs_mgr_do_mount_one() but for overlayfs fstab entries. +// Unlike fs_mgr_overlayfs, mount overlayfs without upperdir and workdir, so the +// filesystem cannot be remount read-write. +bool fs_mgr_mount_overlayfs_fstab_entry(const android::fs_mgr::FstabEntry& entry); diff --git a/fs_mgr/include/fs_mgr_overlayfs.h b/fs_mgr/include/fs_mgr_overlayfs.h index 22d12e753..d45e2de72 100644 --- a/fs_mgr/include/fs_mgr_overlayfs.h +++ b/fs_mgr/include/fs_mgr_overlayfs.h @@ -27,7 +27,6 @@ android::fs_mgr::Fstab fs_mgr_overlayfs_candidate_list(const android::fs_mgr::Fstab& fstab); bool fs_mgr_overlayfs_mount_all(android::fs_mgr::Fstab* fstab); -bool fs_mgr_overlayfs_mount_fstab_entry(const android::fs_mgr::FstabEntry& entry); std::vector fs_mgr_overlayfs_required_devices(android::fs_mgr::Fstab* fstab); bool fs_mgr_overlayfs_setup(const char* backing = nullptr, const char* mount_point = nullptr, bool* change = nullptr, bool force = true); diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index 616d28540..546ea8ed8 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -574,7 +574,7 @@ bool FirstStageMount::MountPartitions() { for (const auto& entry : fstab_) { if (entry.fs_type == "overlay") { - fs_mgr_overlayfs_mount_fstab_entry(entry); + fs_mgr_mount_overlayfs_fstab_entry(entry); } } From aedf0fbce80d30a6d2578cb4d074e0b4d0e48af4 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Mon, 31 May 2021 17:40:49 +0800 Subject: [PATCH 07/10] adb-remount-test: Make awk scripts mawk-v1.3.3-compatible Our CI is failing because the host machine is using mawk instead of gawk. mawk v1.3.3 cannot parse regex such as '/[/]/', while mawk v1.3.4 and gawk can. Change regex of '[/]' to '\/' so that our test script is as backward compatible as possible. Bug: 188862155 Test: Run adb-remount-test.sh on CI Change-Id: Ia4fbce58a61325a5e5280ede0d5b7760832d8ec1 Merged-In: Ia4fbce58a61325a5e5280ede0d5b7760832d8ec1 (cherry picked from commit ad3a57bdbaa05fd531a05373a07f0435a9d5dc33) --- fs_mgr/tests/adb-remount-test.sh | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/fs_mgr/tests/adb-remount-test.sh b/fs_mgr/tests/adb-remount-test.sh index 0c5fe3d65..9542bc167 100755 --- a/fs_mgr/tests/adb-remount-test.sh +++ b/fs_mgr/tests/adb-remount-test.sh @@ -762,15 +762,15 @@ skip_administrative_mounts() { "ramdumpfs" "binder" "securityfs" "functionfs" "rootfs" ) local exclude_devices=( - "[/]sys[/]kernel[/]debug" "[/]data[/]media" "[/]dev[/]block[/]loop[0-9]*" + "\/sys\/kernel\/debug" "\/data\/media" "\/dev\/block\/loop[0-9]*" "${exclude_filesystems[@]}" ) local exclude_mount_points=( - "[/]cache" "[/]mnt[/]scratch" "[/]mnt[/]vendor[/]persist" "[/]persist" - "[/]metadata" + "\/cache" "\/mnt\/scratch" "\/mnt\/vendor\/persist" "\/persist" + "\/metadata" ) if [ "data" = "${1}" ]; then - exclude_mount_points+=("[/]data") + exclude_mount_points+=("\/data") fi awk '$1 !~ /^('"$(join_with "|" "${exclude_devices[@]}")"')$/ && $2 !~ /^('"$(join_with "|" "${exclude_mount_points[@]}")"')$/ && @@ -934,7 +934,7 @@ ACTIVE_SLOT=`get_active_slot` PARTITIONS=`adb_su cat /vendor/etc/fstab* Date: Wed, 19 May 2021 17:34:04 +0800 Subject: [PATCH 08/10] Remove deprecated fs_mgr_overlayfs_required_devices() It is unused since Ifc8720378259654472d3822e97059b6c366f601d Bug: 188862155 Test: Build pass Change-Id: I6304e0dedb36d03a87c556083e937b4a2ce30b1b Merged-In: I6304e0dedb36d03a87c556083e937b4a2ce30b1b (cherry picked from commit a07ed968b563f15b35b68c701a97eb239bb34c4c) --- fs_mgr/fs_mgr_overlayfs.cpp | 4 ---- fs_mgr/include/fs_mgr_overlayfs.h | 1 - 2 files changed, 5 deletions(-) diff --git a/fs_mgr/fs_mgr_overlayfs.cpp b/fs_mgr/fs_mgr_overlayfs.cpp index cb0938363..4d32bda80 100644 --- a/fs_mgr/fs_mgr_overlayfs.cpp +++ b/fs_mgr/fs_mgr_overlayfs.cpp @@ -92,10 +92,6 @@ bool fs_mgr_overlayfs_mount_all(Fstab*) { return false; } -std::vector fs_mgr_overlayfs_required_devices(Fstab*) { - return {}; -} - bool fs_mgr_overlayfs_setup(const char*, const char*, bool* change, bool) { if (change) *change = false; return false; diff --git a/fs_mgr/include/fs_mgr_overlayfs.h b/fs_mgr/include/fs_mgr_overlayfs.h index d45e2de72..6caab1fee 100644 --- a/fs_mgr/include/fs_mgr_overlayfs.h +++ b/fs_mgr/include/fs_mgr_overlayfs.h @@ -27,7 +27,6 @@ android::fs_mgr::Fstab fs_mgr_overlayfs_candidate_list(const android::fs_mgr::Fstab& fstab); bool fs_mgr_overlayfs_mount_all(android::fs_mgr::Fstab* fstab); -std::vector fs_mgr_overlayfs_required_devices(android::fs_mgr::Fstab* fstab); bool fs_mgr_overlayfs_setup(const char* backing = nullptr, const char* mount_point = nullptr, bool* change = nullptr, bool force = true); bool fs_mgr_overlayfs_teardown(const char* mount_point = nullptr, bool* change = nullptr); From a79bc0d1b3c8a89ba4fd529d45e348f57e893688 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Sat, 22 May 2021 00:35:09 +0800 Subject: [PATCH 09/10] fs_mgr_fstab: Parse overlayfs options from fs flags Parse the "lowerdir=" option from fs mount flags instead of fs_mgr flags for consistency. Before: none /mnt/product/test1 overlay ro lowerdir=/1:/2,first_stage_mount After: none /mnt/product/test1 overlay ro,lowerdir=/1:/2 first_stage_mount Bug: 188862155 Test: Boot to normal with overlayfs mount entries in first stage fstab Change-Id: I6d6abd44ab32afadec428005f4aece834f9c8905 Merged-In: I6d6abd44ab32afadec428005f4aece834f9c8905 (cherry picked from commit 7b81023e2a0d48c5e919bb314990555db37bf21e) --- fs_mgr/fs_mgr_fstab.cpp | 21 ++++++++++----------- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/fs_mgr/fs_mgr_fstab.cpp b/fs_mgr/fs_mgr_fstab.cpp index 853b24d8b..d0c89b909 100644 --- a/fs_mgr/fs_mgr_fstab.cpp +++ b/fs_mgr/fs_mgr_fstab.cpp @@ -127,15 +127,16 @@ void ParseMountFlags(const std::string& flags, FstabEntry* entry) { } fs_options.append(flag); - if (entry->fs_type == "f2fs" && StartsWith(flag, "reserve_root=")) { - std::string arg; - if (auto equal_sign = flag.find('='); equal_sign != std::string::npos) { - arg = flag.substr(equal_sign + 1); - } - if (!ParseInt(arg, &entry->reserved_size)) { - LWARNING << "Warning: reserve_root= flag malformed: " << arg; - } else { - entry->reserved_size <<= 12; + if (auto equal_sign = flag.find('='); equal_sign != std::string::npos) { + const auto arg = flag.substr(equal_sign + 1); + if (entry->fs_type == "f2fs" && StartsWith(flag, "reserve_root=")) { + if (!ParseInt(arg, &entry->reserved_size)) { + LWARNING << "Warning: reserve_root= flag malformed: " << arg; + } else { + entry->reserved_size <<= 12; + } + } else if (StartsWith(flag, "lowerdir=")) { + entry->lowerdir = std::move(arg); } } } @@ -298,8 +299,6 @@ void ParseFsMgrFlags(const std::string& flags, FstabEntry* entry) { if (!ParseByteCount(arg, &entry->zram_backingdev_size)) { LWARNING << "Warning: zram_backingdev_size= flag malformed: " << arg; } - } else if (StartsWith(flag, "lowerdir=")) { - entry->lowerdir = arg; } else { LWARNING << "Warning: unknown flag: " << flag; } From 84fe96bfbc1441eaace6b6b88997c2779abf03b6 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Thu, 27 May 2021 22:29:32 +0800 Subject: [PATCH 10/10] first_stage_mount: mount point must be canonical path Ban weird paths such as /../system or //vendor in first stage mount. Add utility function fs_mgr_create_canonical_mount_point() that: * mkdir(mount_point) to ensure mount_point's existence * Test that realpath(mount_point) =?= mount_point Bug: 188898525 Test: Presubmit Test: Boot CF Change-Id: Iaf2ec52701277f26cc81f3e15a47b6083a788334 Merged-In: Iaf2ec52701277f26cc81f3e15a47b6083a788334 (cherry picked from commit 3431d52675f25020b279c9fbcda6b8648f9cf67b) --- fs_mgr/fs_mgr.cpp | 33 +++++++++++++++++++++------------ fs_mgr/include/fs_mgr.h | 4 ++++ init/first_stage_mount.cpp | 4 ++++ 3 files changed, 29 insertions(+), 12 deletions(-) diff --git a/fs_mgr/fs_mgr.cpp b/fs_mgr/fs_mgr.cpp index 08ead7a06..af71fe6d5 100644 --- a/fs_mgr/fs_mgr.cpp +++ b/fs_mgr/fs_mgr.cpp @@ -2266,6 +2266,26 @@ std::string fs_mgr_get_super_partition_name(int slot) { return LP_METADATA_DEFAULT_PARTITION_NAME; } +bool fs_mgr_create_canonical_mount_point(const std::string& mount_point) { + auto saved_errno = errno; + auto ok = true; + auto created_mount_point = !mkdir(mount_point.c_str(), 0755); + std::string real_mount_point; + if (!Realpath(mount_point, &real_mount_point)) { + ok = false; + PERROR << "failed to realpath(" << mount_point << ")"; + } else if (mount_point != real_mount_point) { + ok = false; + LERROR << "mount point is not canonical: realpath(" << mount_point << ") -> " + << real_mount_point; + } + if (!ok && created_mount_point) { + rmdir(mount_point.c_str()); + } + errno = saved_errno; + return ok; +} + bool fs_mgr_mount_overlayfs_fstab_entry(const FstabEntry& entry) { auto overlayfs_valid_result = fs_mgr_overlayfs_valid(); if (overlayfs_valid_result == OverlayfsValidResult::kNotSupported) { @@ -2298,18 +2318,7 @@ bool fs_mgr_mount_overlayfs_fstab_entry(const FstabEntry& entry) { } #endif // ALLOW_ADBD_DISABLE_VERITY == 0 - // Create the mount point in case it doesn't exist. - mkdir(entry.mount_point.c_str(), 0755); - - // Ensure that mount point exists and doesn't contain symbolic link or /../. - std::string mount_point; - if (!Realpath(entry.mount_point, &mount_point)) { - PERROR << __FUNCTION__ << "(): failed to realpath " << entry.mount_point; - return false; - } - if (entry.mount_point != mount_point) { - LERROR << __FUNCTION__ << "(): mount point must be a canonicalized path: realpath " - << entry.mount_point << " = " << mount_point; + if (!fs_mgr_create_canonical_mount_point(entry.mount_point)) { return false; } diff --git a/fs_mgr/include/fs_mgr.h b/fs_mgr/include/fs_mgr.h index b8ebd63a4..4d3ecc9dc 100644 --- a/fs_mgr/include/fs_mgr.h +++ b/fs_mgr/include/fs_mgr.h @@ -132,6 +132,10 @@ int fs_mgr_remount_userdata_into_checkpointing(android::fs_mgr::Fstab* fstab); // empty string std::string fs_mgr_find_bow_device(const std::string& block_device); +// Creates mount point if not already existed, and checks that mount point is a +// canonical path that doesn't contain any symbolic link or /../. +bool fs_mgr_create_canonical_mount_point(const std::string& mount_point); + // Like fs_mgr_do_mount_one() but for overlayfs fstab entries. // Unlike fs_mgr_overlayfs, mount overlayfs without upperdir and workdir, so the // filesystem cannot be remount read-write. diff --git a/init/first_stage_mount.cpp b/init/first_stage_mount.cpp index 546ea8ed8..f5c10bbd4 100644 --- a/init/first_stage_mount.cpp +++ b/init/first_stage_mount.cpp @@ -420,6 +420,10 @@ bool FirstStageMount::MountPartition(const Fstab::iterator& begin, bool erase_sa *end = begin + 1; } + if (!fs_mgr_create_canonical_mount_point(begin->mount_point)) { + return false; + } + if (begin->fs_mgr_flags.logical) { if (!fs_mgr_update_logical_partition(&(*begin))) { return false;