From a16e437e9d86e3e2b01099a75fd919d4902363fa Mon Sep 17 00:00:00 2001 From: Mark Salyzyn Date: Wed, 20 Sep 2017 08:36:12 -0700 Subject: [PATCH] bootstat: validate last kmsg and bootreason content more carefully Establish a tighter trust relationship with digitization of the battery level. Validate the reboot reason collected from the last kmsg. Validate the last reboot reason before using for enhancing system reboot reason. Test: system/core/bootstat/boot_reason_test.sh Bug: 63636262 Change-Id: I45fbac4a33c09295cda89de3b73c93a884033e3b --- bootstat/boot_reason_test.sh | 35 ++++++++++++++++++++++++++++++----- bootstat/bootstat.cpp | 36 +++++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 12 deletions(-) diff --git a/bootstat/boot_reason_test.sh b/bootstat/boot_reason_test.sh index 4314271da..b76011a04 100755 --- a/bootstat/boot_reason_test.sh +++ b/bootstat/boot_reason_test.sh @@ -370,6 +370,7 @@ test_ota() { fast and fake (touch build_date on device to make it different)" ] test_optional_ota() { echo "INFO: expected duration of ${TEST} test about 45 seconds" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 adb shell su root touch /data/misc/bootstat/build_date >&2 adb reboot ota wait_for_screen @@ -425,6 +426,7 @@ Decision to rummage through bootstat data files was made as a _real_ factory_reset is too destructive to the device." ] test_factory_reset() { echo "INFO: expected duration of ${TEST} test roughly 45 seconds" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 adb shell su root rm /data/misc/bootstat/build_date >&2 adb reboot >&2 wait_for_screen @@ -479,7 +481,8 @@ test_hard() { [ "USAGE: test_battery battery test (trick): -- echo healthd: battery l=2 | adb shell su root tee /dev/kmsg ; adb reboot cold +- echo healthd: battery l=2 | adb shell su root tee /dev/kmsg +- adb reboot cold - (wait until screen is up, boot has completed) - adb shell getprop sys.boot.reason - NB: should report reboot,battery, unless healthd managed to log @@ -491,7 +494,7 @@ battery test (trick): + setprop logd.kernel false + rm /sys/fs/pstore/console-ramoops + rm /sys/fs/pstore/console-ramoops-0 - + write /dev/kmsg \"healthd: battery l=2 + + write /dev/kmsg \"healthd: battery l=2${SPACE} +\" - adb reboot fs - (wait until screen is up, boot has completed) @@ -500,9 +503,10 @@ battery test (trick): - (replace set logd.kernel true to the above, and retry test)" ] test_battery() { echo "INFO: expected duration of ${TEST} test roughly two minutes" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 # Send it _many_ times to combat devices with flakey pstore for i in a b c d e f g h i j k l m n o p q r s t u v w x y z; do - echo healthd: battery l=2 | adb shell su root tee /dev/kmsg >/dev/null + echo 'healthd: battery l=2 ' | adb shell su root tee /dev/kmsg >/dev/null done adb reboot cold >&2 adb wait-for-device @@ -512,11 +516,11 @@ test_battery() { /proc/fs/pstore/console-ramoops-0 2>/dev/null | grep 'healthd: battery l=' | tail -1 | - grep 'healthd: battery l=2' >/dev/null || ( + grep 'healthd: battery l=2 ' >/dev/null || ( if ! EXPECT_PROPERTY sys.boot.reason reboot,battery >/dev/null 2>/dev/null; then # retry for i in a b c d e f g h i j k l m n o p q r s t u v w x y z; do - echo healthd: battery l=2 | adb shell su root tee /dev/kmsg >/dev/null + echo 'healthd: battery l=2 ' | adb shell su root tee /dev/kmsg >/dev/null done adb reboot cold >&2 adb wait-for-device @@ -550,6 +554,7 @@ kernel_panic test: - NB: should report kernel_panic,sysrq" ] test_kernel_panic() { echo "INFO: expected duration of ${TEST} test > 2 minutes" >&2 + echo "WARNING: ${TEST} requires userdebug build" >&2 echo c | adb shell su root tee /proc/sysrq-trigger >/dev/null wait_for_screen EXPECT_PROPERTY sys.boot.reason kernel_panic,sysrq @@ -640,6 +645,26 @@ test_adb_reboot() { report_bootstat_logs reboot,adb } +[ "USAGE: test_Its_Just_So_Hard_reboot + +Its Just So Hard reboot test: +- adb shell reboot 'Its Just So Hard' +- (wait until screen is up, boot has completed) +- adb shell getprop sys.boot.reason +- NB: should report reboot,its_just_so_hard +- NB: expect log \"... I bootstat: Unknown boot reason: reboot,its_just_so_hard\"" ] +test_Its_Just_So_Hard_reboot() { + echo "INFO: expected duration of ${TEST} test roughly 45 seconds" >&2 + echo "INFO: ${TEST} cleanup requires userdebug build" >&2 + adb shell 'reboot "Its Just So Hard"' + wait_for_screen + EXPECT_PROPERTY sys.boot.reason reboot,its_just_so_hard + EXPECT_PROPERTY persist.sys.boot.reason "reboot,Its Just So Hard" + adb shell su root setprop persist.sys.boot.reason reboot,its_just_so_hard + EXPECT_PROPERTY persist.sys.boot.reason reboot,its_just_so_hard + report_bootstat_logs reboot,its_just_so_hard +} + [ "USAGE: ${0##*/} [-s SERIAL] [tests] Mainline executive to run the above tests" ] diff --git a/bootstat/bootstat.cpp b/bootstat/bootstat.cpp index ce28059be..e2202fdef 100644 --- a/bootstat/bootstat.cpp +++ b/bootstat/bootstat.cpp @@ -334,6 +334,8 @@ const char bootloader_reboot_reason_property[] = "ro.boot.bootreason"; // Scrub, Sanitize, Standardize and Enhance the boot reason string supplied. std::string BootReasonStrToReason(const std::string& boot_reason) { + static const size_t max_reason_length = 256; + std::string ret(GetProperty(system_reboot_reason_property)); std::string reason(boot_reason); // If sys.boot.reason == ro.boot.bootreason, let's re-evaluate @@ -428,9 +430,15 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { size_t pos = content.rfind(cmd); if (pos != std::string::npos) { pos += strlen(cmd); - std::string subReason(content.substr(pos)); - pos = subReason.find('\''); - if (pos != std::string::npos) subReason.erase(pos); + std::string subReason(content.substr(pos, max_reason_length)); + for (pos = 0; pos < subReason.length(); ++pos) { + char c = tounderline(subReason[pos]); + if (!::isprint(c) || (c == '\'')) { + subReason.erase(pos); + break; + } + subReason[pos] = ::tolower(c); + } if (subReason != "") { // Will not land "reboot" as that is too blunt. if (isKernelRebootReason(subReason)) { ret = "reboot," + subReason; // User space can't talk kernel reasons. @@ -461,13 +469,20 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { static const int battery_dead_threshold = 2; // percent static const char battery[] = "healthd: battery l="; size_t pos = content.rfind(battery); // last one + std::string digits; if (pos != std::string::npos) { - int level = atoi(content.substr(pos + strlen(battery)).c_str()); + digits = content.substr(pos + strlen(battery)); + } + char* endptr = NULL; + unsigned long long level = strtoull(digits.c_str(), &endptr, 10); + if ((level <= 100) && (endptr != digits.c_str()) && (*endptr == ' ')) { LOG(INFO) << "Battery level at shutdown " << level << "%"; if (level <= battery_dead_threshold) { ret = "shutdown,battery"; } - } else { // Most likely + } else { // Most likely + digits = ""; // reset digits + // Content buffer no longer will have console data. Beware if more // checks added below, that depend on parsing console content. content = ""; @@ -501,8 +516,11 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { pos = content.find(match); // The first one it finds. if (pos != std::string::npos) { - pos += strlen(match); - int level = atoi(content.substr(pos).c_str()); + digits = content.substr(pos + strlen(match)); + } + endptr = NULL; + level = strtoull(digits.c_str(), &endptr, 10); + if ((level <= 100) && (endptr != digits.c_str()) && (*endptr == ' ')) { LOG(INFO) << "Battery level at startup " << level << "%"; if (level <= battery_dead_threshold) { ret = "shutdown,battery"; @@ -518,6 +536,10 @@ std::string BootReasonStrToReason(const std::string& boot_reason) { // Content buffer no longer will have console data. Beware if more // checks added below, that depend on parsing console content. content = GetProperty(last_reboot_reason_property); + // Cleanup last_boot_reason regarding acceptable character set + std::transform(content.begin(), content.end(), content.begin(), ::tolower); + std::transform(content.begin(), content.end(), content.begin(), tounderline); + std::transform(content.begin(), content.end(), content.begin(), toprintable); // String is either "reboot," or "shutdown,". // We will set if default reasons, only override with detail if thermal.