From 659c96b12e58e081f159128c0a83380b1689fbe9 Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Sat, 27 Aug 2022 14:02:07 +0800 Subject: [PATCH 1/3] adb-remount-test: Miscellaneous fixes * When guessing the ANDROID_SERIAL, use output of `adb devices` instead of ro.serialno, because ro.serialno won't work for network devices. * Ensure ANDROID_SERIAL is exported so the test don't fail if a new device is plugged into the host machine mid test. * Change --wait-screen warning to info. The "warning" isn't helpful as it's not showing any potential problems. * Register cleanup hooks to EXIT trap. This ensures cleanup code are always executed, and failure to clean up counts as test failure. * Rewrite some unnecessarily complex command chaining to plain exit status check. * Use `test` command to test file existence. Don't use `ls` or `cat` to test file as this isn't their intended usage, and parsing their error output can be finicky. Bug: 243116800 Fixes: 178256393 Test: adb-remount-test Change-Id: Iec4224d8a236a9852ce417b1129c27205d435d5b --- fs_mgr/tests/adb-remount-test.sh | 99 +++++++++++++------------------- 1 file changed, 41 insertions(+), 58 deletions(-) diff --git a/fs_mgr/tests/adb-remount-test.sh b/fs_mgr/tests/adb-remount-test.sh index a1e94ddc1..f522674e1 100755 --- a/fs_mgr/tests/adb-remount-test.sh +++ b/fs_mgr/tests/adb-remount-test.sh @@ -231,17 +231,6 @@ adb_cat() { return ${ret} } -[ "USAGE: adb_ls >stdout - -Returns: filename or directoru content to stdout with carriage returns skipped, - true if the ls had no errors" ] -adb_ls() { - local OUTPUT="`adb_sh ls ${1} /dev/null`" - local ret=${?} - echo "${OUTPUT}" | tr -d '\r' - return ${ret} -} - [ "USAGE: adb_test Returns: exit status of the test expression" ] @@ -648,9 +637,6 @@ die() { shift 2 fi >&2 LOG FAILED "${@}" - cleanup - restore - test_duration exit 1 } @@ -885,10 +871,32 @@ if ! ${color}; then NORMAL="" fi +exit_handler() { + cleanup || true + local err=0 + if ! restore; then + LOG ERROR "restore failed" + err=1 + fi >&2 + test_duration || true + if [ "${err}" != 0 ]; then + exit "${err}" + fi +} +trap 'exit_handler' EXIT + if ${print_time}; then LOG INFO "start $(date)" fi +if [ -z "${ANDROID_SERIAL}" ]; then + inAdb || die "no device or more than one device in adb mode" + D=$(adb devices | awk '$2 == "device" { print $1; exit }') + [ -n "${D}" ] || die "cannot get device serial" + ANDROID_SERIAL="${D}" +fi +export ANDROID_SERIAL + inFastboot && die "device in fastboot mode" inRecovery && die "device in recovery mode" if ! inAdb; then @@ -913,9 +921,6 @@ fi # Collect characteristics of the device and report. -D=`get_property ro.serialno` -[ -n "${D}" ] || D=`get_property ro.boot.serialno` -[ -z "${D}" -o -n "${ANDROID_SERIAL}" ] || ANDROID_SERIAL=${D} USB_SERIAL= if [ -n "${ANDROID_SERIAL}" -a "Darwin" != "${HOSTOS}" ]; then USB_SERIAL="`find /sys/devices -name serial | grep usb || true`" @@ -929,8 +934,8 @@ if [ -n "${USB_SERIAL}" ]; then USB_ADDRESS=${USB_SERIAL%/serial} USB_ADDRESS=usb${USB_ADDRESS##*/} fi -[ -z "${ANDROID_SERIAL}${USB_ADDRESS}" ] || - USB_DEVICE=`usb_devnum` +USB_DEVICE=$(usb_devnum) +[ -z "${ANDROID_SERIAL}${USB_ADDRESS}${USB_DEVICE}" ] || LOG INFO "${ANDROID_SERIAL} ${USB_ADDRESS} ${USB_DEVICE}" BUILD_DESCRIPTION=`get_property ro.build.description` [ -z "${BUILD_DESCRIPTION}" ] || @@ -1042,7 +1047,7 @@ restore() { # If reboot too soon after fresh flash, could trip device update failure logic if ${screen_wait}; then - LOG WARNING "waiting for screen to come up. Consider --no-wait-screen option" + LOG INFO "waiting for screen to come up. Consider --no-wait-screen option" fi if ! wait_for_screen && ${screen_wait}; then screen_wait=false @@ -1156,11 +1161,7 @@ adb_unroot adb_sh find ${MOUNTS} /dev/null 2>/dev/null || true adb_root -D=`adb remount 2>&1` -ret=${?} -echo "${D}" >&2 -[ ${ret} != 0 ] || - [ X"${D}" = X"${D##*remount failed}" ] || +adb remount >&2 || die -t "${T}" "adb remount failed" D=`adb_sh df -k /dev/null 2>/dev/null || true fi # If overlayfs has a nested security problem, this will fail. -B="`adb_ls /system/`" || - die "adb ls /system" -[ X"${B}" != X"${B#*priv-app}" ] || - die "adb ls /system/priv-app" +adb_sh ls /system >/dev/null || die "ls /system" +adb_test -d /system/priv-app || die "[ -d /system/priv-app ]" B="`adb_cat /system/priv-app/hello`" check_eq "${A}" "${B}" /system/priv-app after reboot # Only root can read vendor if sepolicy permissions are as expected. @@ -1448,22 +1447,19 @@ else fi B="`adb_cat /system/hello`" check_eq "${A}" "${B}" system after flash vendor - B="`adb_ls /system/`" || - die "adb ls /system" - [ X"${B}" != X"${B#*priv-app}" ] || - die "adb ls /system/priv-app" + adb_sh ls /system >/dev/null || die "ls /system" + adb_test -d /system/priv-app || die "[ -d /system/priv-app ]" B="`adb_cat /system/priv-app/hello`" check_eq "${A}" "${B}" system/priv-app after flash vendor adb_root || die "adb root" - B="`adb_cat /vendor/hello`" if ${is_userspace_fastboot} || ! ${overlayfs_needed}; then - check_eq "cat: /vendor/hello: No such file or directory" "${B}" \ - vendor content after flash vendor + adb_test -e /vendor/hello && + die "vendor content after flash vendor" else LOG WARNING "user fastboot missing required to invalidate, ignoring a failure" - check_eq "cat: /vendor/hello: No such file or directory" "${B}" \ - --warning vendor content after flash vendor + adb_test -e /vendor/hello && + LOG WARNING "vendor content after flash vendor" fi check_eq "${SYSTEM_INO}" "`adb_sh stat --format=%i /system/hello &2 adb_sh rm /system/hello /system/priv-app/hello &2 || die -t ${T} "cleanup hello" -B="`adb_cat /system/hello`" -check_eq "cat: /system/hello: No such file or directory" "${B}" after rm -B="`adb_cat /system/priv-app/hello`" -check_eq "cat: /system/priv-app/hello: No such file or directory" "${B}" after rm -B="`adb_cat /vendor/hello`" -check_eq "cat: /vendor/hello: No such file or directory" "${B}" after rm +adb_test -e /system/hello && + die "/system/hello lingers after rm" +adb_test -e /system/priv-app/hello && + die "/system/priv-app/hello lingers after rm" +adb_test -e /vendor/hello && + die "/vendor/hello lingers after rm" for i in ${MOUNTS}; do adb_sh rm ${i}/hello /dev/null || true done @@ -1554,12 +1550,7 @@ if ${is_bootloader_fastboot} && [ -n "${scratch_partition}" ]; then [ X"${D}" != X"${D##*[Uu]sing overlayfs}" ] && LOG OK "${scratch_partition} recreated" || die -t ${T} "setup for overlayfs" - D=`adb remount 2>&1` - err=${?} - echo "${D}" >&2 - [ ${err} != 0 ] || - [ X"${D}" = X"${D##*remount failed}" ] || - ( echo "${D}" && false ) >&2 || + adb remount >&2 || die -t ${T} "remount failed" fi @@ -1627,13 +1618,5 @@ adb_sh grep " \(/system\|/\) .* rw," /proc/mounts >/dev/null Date: Sat, 27 Aug 2022 14:02:07 +0800 Subject: [PATCH 2/3] adb-remount-test: Reduce noise from initial overlayfs diagnostics There's quite a lot of noise from running "Checking current overlayfs status". Improve the test output by filtering uninteresting df lines. * "/apex/..." mounts not interesting. * "rw" mounts not interesting. * "fuse" devices not interesting. Bug: 243116800 Test: adb-remount-test Change-Id: Id15844d853aaf3f7ed86f1a83544494b697b5b39 --- fs_mgr/tests/adb-remount-test.sh | 24 ++++++++++++------------ 1 file changed, 12 insertions(+), 12 deletions(-) diff --git a/fs_mgr/tests/adb-remount-test.sh b/fs_mgr/tests/adb-remount-test.sh index f522674e1..5e06064cf 100755 --- a/fs_mgr/tests/adb-remount-test.sh +++ b/fs_mgr/tests/adb-remount-test.sh @@ -721,7 +721,7 @@ 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" + "ramdumpfs" "binder" "securityfs" "functionfs" "rootfs" "fuse" ) local exclude_devices=( "\/sys\/kernel\/debug" "\/data\/media" "\/dev\/block\/loop[0-9]*" @@ -746,8 +746,10 @@ or output from df Filters out all apex and vendor override administrative overlay mounts uninteresting to the test" ] skip_unrelated_mounts() { - grep -v "^overlay.* /\(apex\|bionic\|system\|vendor\)/[^ ]" | - grep -v "[%] /\(data_mirror\|apex\|bionic\|system\|vendor\)/[^ ][^ ]*$" + grep -vE \ + -e "^overlay.* /(apex|bionic|system|vendor)/[^ ]" \ + -e "^[^ ]+ /apex/[^ ]" \ + -e "[%] /(data_mirror|apex|bionic|system|vendor)/[^ ]+$" } [ "USAGE: surgically_wipe_overlayfs @@ -1077,21 +1079,19 @@ is_overlayfs_mounted && die "overlay takeover unexpected at this phase" overlayfs_needed=true -D=`adb_sh cat /proc/mounts /dev/null; then - D=`echo / / - echo "${D}" | grep -v /dev/root` -fi -D=`echo "${D}" | cut -s -d' ' -f1 | sort -u` +D=$(adb_sh grep " ro," /proc/mounts &1 | grep "Filesystem features:.*shared_blocks" >/dev/null && no_dedupe=false done -D=`adb_sh df -k ${D} &2 if [ X"${D}" = X"${D##* 100[%] }" ] && ${no_dedupe} ; then overlayfs_needed=false From 9a636d2a850482a0fc7a694bc282e24c23dfab6d Mon Sep 17 00:00:00 2001 From: Yi-Yo Chiang Date: Sat, 27 Aug 2022 14:02:07 +0800 Subject: [PATCH 3/3] adb-remount-test: Discover fstab pathname more intelligently Pick exactly one fstab file whose pathname suffix matches one of the fstab suffix properties. This helps on CF who ships redundant copies of fstab. Bug: 243116800 Test: adb-remount-test Change-Id: I4d38859014161e14dba1f7e19dbce44a2621d0f1 --- fs_mgr/tests/adb-remount-test.sh | 21 +++++++++++++++------ 1 file changed, 15 insertions(+), 6 deletions(-) diff --git a/fs_mgr/tests/adb-remount-test.sh b/fs_mgr/tests/adb-remount-test.sh index 5e06064cf..e6d55d9fa 100755 --- a/fs_mgr/tests/adb-remount-test.sh +++ b/fs_mgr/tests/adb-remount-test.sh @@ -950,14 +950,23 @@ ACTIVE_SLOT=`get_active_slot` LOG INFO "active slot is ${ACTIVE_SLOT}" # Acquire list of system partitions +FSTAB_SUFFIXES=( + "$(get_property ro.boot.fstab_suffix)" + "$(get_property ro.boot.hardware)" + "$(get_property ro.boot.hardware.platform)" +) +FSTAB_PATTERN='\.('"$(join_with "|" "${FSTAB_SUFFIXES[@]}")"')$' +FSTAB_FILE=$(adb_su ls -1 '/vendor/etc/fstab*' ") -PARTITIONS=`adb_su cat /vendor/etc/fstab*