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
diff --git a/bootstat/boot_reason_test.sh b/bootstat/boot_reason_test.sh
index 4314271..b76011a 100755
--- a/bootstat/boot_reason_test.sh
+++ b/bootstat/boot_reason_test.sh
@@ -370,6 +370,7 @@
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 @@
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 @@
[ "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<space> | 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 @@
+ 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 @@
- (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 @@
/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 @@
- 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 @@
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 ce28059..e2202fd 100644
--- a/bootstat/bootstat.cpp
+++ b/bootstat/bootstat.cpp
@@ -334,6 +334,8 @@
// 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 @@
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 @@
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 @@
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 @@
// 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,<reason>" or "shutdown,<reason>".
// We will set if default reasons, only override with detail if thermal.