diff --git a/crash_reporter/Makefile b/crash_reporter/Makefile index b51743d35..fc2d68013 100644 --- a/crash_reporter/Makefile +++ b/crash_reporter/Makefile @@ -4,9 +4,18 @@ CRASH_REPORTER = crash_reporter REPORTER_BINS = $(CRASH_REPORTER) -CRASH_OBJS = system_logging.o user_collector.o +CRASH_OBJS = \ + crash_collector.o \ + kernel_collector.o \ + system_logging.o \ + unclean_shutdown_collector.o \ + user_collector.o TEST_OBJS = $(CRASH_OBJS) system_logging_mock.o -TEST_BINS = user_collector_test +TEST_BINS = \ + crash_collector_test \ + kernel_collector_test \ + unclean_shutdown_collector_test \ + user_collector_test # -lglib-2.0 is needed by libbase.a now. COMMON_LIBS = -lbase -lpthread -lglib-2.0 -lgflags -lrt @@ -24,7 +33,7 @@ $(CRASH_REPORTER): crash_reporter.o $(CRASH_OBJS) tests: $(TEST_BINS) -user_collector_test: user_collector_test.o $(TEST_OBJS) +%_test: %_test.o $(TEST_OBJS) $(CXX) $(CXXFLAGS) $(LIB_DIRS) $^ $(TEST_LIBS) -o $@ .cc.o: diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc new file mode 100644 index 000000000..492f2e8bc --- /dev/null +++ b/crash_reporter/crash_collector.cc @@ -0,0 +1,150 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "crash-reporter/crash_collector.h" + +#include // For struct passwd. +#include // for mode_t. + +#include "base/file_util.h" +#include "base/logging.h" +#include "base/string_util.h" +#include "crash-reporter/system_logging.h" + +static const char kDefaultUserName[] = "chronos"; +static const char kSystemCrashPath[] = "/var/spool/crash"; +static const char kUserCrashPath[] = "/home/chronos/user/crash"; + +// Directory mode of the user crash spool directory. +static const mode_t kUserCrashPathMode = 0755; + +// Directory mode of the system crash spool directory. +static const mode_t kSystemCrashPathMode = 01755; + +static const uid_t kRootOwner = 0; +static const uid_t kRootGroup = 0; + +CrashCollector::CrashCollector() : forced_crash_directory_(NULL) { +} + +CrashCollector::~CrashCollector() { +} + +void CrashCollector::Initialize( + CrashCollector::CountCrashFunction count_crash_function, + CrashCollector::IsFeedbackAllowedFunction is_feedback_allowed_function, + SystemLogging *logger) { + CHECK(count_crash_function != NULL); + CHECK(is_feedback_allowed_function != NULL); + CHECK(logger != NULL); + + count_crash_function_ = count_crash_function; + is_feedback_allowed_function_ = is_feedback_allowed_function; + logger_ = logger; +} + +std::string CrashCollector::FormatDumpBasename(const std::string &exec_name, + time_t timestamp, + pid_t pid) { + struct tm tm; + localtime_r(×tamp, &tm); + return StringPrintf("%s.%04d%02d%02d.%02d%02d%02d.%d", + exec_name.c_str(), + tm.tm_year + 1900, + tm.tm_mon + 1, + tm.tm_mday, + tm.tm_hour, + tm.tm_min, + tm.tm_sec, + pid); +} + +FilePath CrashCollector::GetCrashDirectoryInfo( + uid_t process_euid, + uid_t default_user_id, + gid_t default_user_group, + mode_t *mode, + uid_t *directory_owner, + gid_t *directory_group) { + if (process_euid == default_user_id) { + *mode = kUserCrashPathMode; + *directory_owner = default_user_id; + *directory_group = default_user_group; + return FilePath(kUserCrashPath); + } else { + *mode = kSystemCrashPathMode; + *directory_owner = kRootOwner; + *directory_group = kRootGroup; + return FilePath(kSystemCrashPath); + } +} + +bool CrashCollector::GetUserInfoFromName(const std::string &name, + uid_t *uid, + gid_t *gid) { + char storage[256]; + struct passwd passwd_storage; + struct passwd *passwd_result = NULL; + + if (getpwnam_r(name.c_str(), &passwd_storage, storage, sizeof(storage), + &passwd_result) != 0 || passwd_result == NULL) { + logger_->LogError("Cannot find user named %s", name.c_str()); + return false; + } + + *uid = passwd_result->pw_uid; + *gid = passwd_result->pw_gid; + return true; +} + +bool CrashCollector::GetCreatedCrashDirectoryByEuid(uid_t euid, + FilePath *crash_directory) { + uid_t default_user_id; + gid_t default_user_group; + + // For testing. + if (forced_crash_directory_ != NULL) { + *crash_directory = FilePath(forced_crash_directory_); + return true; + } + + if (!GetUserInfoFromName(kDefaultUserName, + &default_user_id, + &default_user_group)) { + logger_->LogError("Could not find default user info"); + return false; + } + mode_t directory_mode; + uid_t directory_owner; + gid_t directory_group; + *crash_directory = + GetCrashDirectoryInfo(euid, + default_user_id, + default_user_group, + &directory_mode, + &directory_owner, + &directory_group); + + if (!file_util::PathExists(*crash_directory)) { + // Create the spool directory with the appropriate mode (regardless of + // umask) and ownership. + mode_t old_mask = umask(0); + if (mkdir(crash_directory->value().c_str(), directory_mode) < 0 || + chown(crash_directory->value().c_str(), + directory_owner, + directory_group) < 0) { + logger_->LogError("Unable to create appropriate crash directory"); + return false; + } + umask(old_mask); + } + + if (!file_util::PathExists(*crash_directory)) { + logger_->LogError("Unable to create crash directory %s", + crash_directory->value().c_str()); + return false; + } + + return true; +} diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h new file mode 100644 index 000000000..ef0d2a7b1 --- /dev/null +++ b/crash_reporter/crash_collector.h @@ -0,0 +1,70 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef _CRASH_REPORTER_CRASH_COLLECTOR_H_ +#define _CRASH_REPORTER_CRASH_COLLECTOR_H_ + +#include +#include + +#include "gtest/gtest_prod.h" // for FRIEND_TEST + +class FilePath; +class SystemLogging; + +// User crash collector. +class CrashCollector { + public: + typedef void (*CountCrashFunction)(); + typedef bool (*IsFeedbackAllowedFunction)(); + + CrashCollector(); + + virtual ~CrashCollector(); + + // Initialize the crash collector for detection of crashes, given a + // crash counting function, metrics collection enabled oracle, and + // system logger facility. + void Initialize(CountCrashFunction count_crash, + IsFeedbackAllowedFunction is_metrics_allowed, + SystemLogging *logger); + + protected: + friend class CrashCollectorTest; + FRIEND_TEST(CrashCollectorTest, GetCrashDirectoryInfo); + FRIEND_TEST(CrashCollectorTest, FormatDumpBasename); + FRIEND_TEST(CrashCollectorTest, Initialize); + + // For testing, set the directory always returned by + // GetCreatedCrashDirectoryByEuid. + void ForceCrashDirectory(const char *forced_directory) { + forced_crash_directory_ = forced_directory; + } + + FilePath GetCrashDirectoryInfo(uid_t process_euid, + uid_t default_user_id, + gid_t default_user_group, + mode_t *mode, + uid_t *directory_owner, + gid_t *directory_group); + bool GetUserInfoFromName(const std::string &name, + uid_t *uid, + gid_t *gid); + // Determines the crash directory for given eud, and creates the + // directory if necessary with appropriate permissions. Returns + // true whether or not directory needed to be created, false on any + // failure. + bool GetCreatedCrashDirectoryByEuid(uid_t euid, + FilePath *crash_file_path); + std::string FormatDumpBasename(const std::string &exec_name, + time_t timestamp, + pid_t pid); + + CountCrashFunction count_crash_function_; + IsFeedbackAllowedFunction is_feedback_allowed_function_; + SystemLogging *logger_; + const char *forced_crash_directory_; +}; + +#endif // _CRASH_REPORTER_CRASH_COLLECTOR_H_ diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc new file mode 100644 index 000000000..48b7fa03c --- /dev/null +++ b/crash_reporter/crash_collector_test.cc @@ -0,0 +1,105 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "base/file_util.h" +#include "crash-reporter/crash_collector.h" +#include "crash-reporter/system_logging_mock.h" +#include "gflags/gflags.h" +#include "gtest/gtest.h" + +void CountCrash() { + ADD_FAILURE(); +} + +bool IsMetrics() { + ADD_FAILURE(); + return false; +} + +class CrashCollectorTest : public ::testing::Test { + void SetUp() { + collector_.Initialize(CountCrash, + IsMetrics, + &logging_); + } + protected: + SystemLoggingMock logging_; + CrashCollector collector_; + pid_t pid_; +}; + +TEST_F(CrashCollectorTest, Initialize) { + ASSERT_TRUE(CountCrash == collector_.count_crash_function_); + ASSERT_TRUE(IsMetrics == collector_.is_feedback_allowed_function_); + ASSERT_TRUE(&logging_ == collector_.logger_); +} + +TEST_F(CrashCollectorTest, GetCrashDirectoryInfo) { + FilePath path; + const int kRootUid = 0; + const int kRootGid = 0; + const int kNtpUid = 5; + const int kChronosUid = 1000; + const int kChronosGid = 1001; + const mode_t kExpectedSystemMode = 01755; + const mode_t kExpectedUserMode = 0755; + + mode_t directory_mode; + uid_t directory_owner; + gid_t directory_group; + + path = collector_.GetCrashDirectoryInfo(kRootUid, + kChronosUid, + kChronosGid, + &directory_mode, + &directory_owner, + &directory_group); + EXPECT_EQ("/var/spool/crash", path.value()); + EXPECT_EQ(kExpectedSystemMode, directory_mode); + EXPECT_EQ(kRootUid, directory_owner); + EXPECT_EQ(kRootGid, directory_group); + + path = collector_.GetCrashDirectoryInfo(kNtpUid, + kChronosUid, + kChronosGid, + &directory_mode, + &directory_owner, + &directory_group); + EXPECT_EQ("/var/spool/crash", path.value()); + EXPECT_EQ(kExpectedSystemMode, directory_mode); + EXPECT_EQ(kRootUid, directory_owner); + EXPECT_EQ(kRootGid, directory_group); + + path = collector_.GetCrashDirectoryInfo(kChronosUid, + kChronosUid, + kChronosGid, + &directory_mode, + &directory_owner, + &directory_group); + EXPECT_EQ("/home/chronos/user/crash", path.value()); + EXPECT_EQ(kExpectedUserMode, directory_mode); + EXPECT_EQ(kChronosUid, directory_owner); + EXPECT_EQ(kChronosGid, directory_group); +} + +TEST_F(CrashCollectorTest, FormatDumpBasename) { + struct tm tm = {0}; + tm.tm_sec = 15; + tm.tm_min = 50; + tm.tm_hour = 13; + tm.tm_mday = 23; + tm.tm_mon = 4; + tm.tm_year = 110; + tm.tm_isdst = -1; + std::string basename = + collector_.FormatDumpBasename("foo", mktime(&tm), 100); + ASSERT_EQ("foo.20100523.135015.100", basename); +} + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc index 0930cea44..e850e3070 100644 --- a/crash_reporter/crash_reporter.cc +++ b/crash_reporter/crash_reporter.cc @@ -7,7 +7,9 @@ #include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" +#include "crash-reporter/kernel_collector.h" #include "crash-reporter/system_logging.h" +#include "crash-reporter/unclean_shutdown_collector.h" #include "crash-reporter/user_collector.h" #include "gflags/gflags.h" #include "metrics/metrics_library.h" @@ -22,69 +24,52 @@ DEFINE_bool(unclean_check, true, "Check for unclean shutdown"); #pragma GCC diagnostic error "-Wstrict-aliasing" static const char kCrashCounterHistogram[] = "Logging.CrashCounter"; -static const char kEmpty[] = ""; +static const char kUserCrashSignal[] = + "org.chromium.CrashReporter.UserCrash"; static const char kUncleanShutdownFile[] = "/var/lib/crash_reporter/pending_clean_shutdown"; // Enumeration of kinds of crashes to be used in the CrashCounter histogram. enum CrashKinds { - kCrashKindKernel = 1, - kCrashKindUser = 2, + kCrashKindUncleanShutdown = 1, + kCrashKindUser = 2, + kCrashKindKernel = 3, kCrashKindMax }; static MetricsLibrary s_metrics_lib; static SystemLoggingImpl s_system_log; -static bool IsMetricsCollectionAllowed() { - // TODO(kmixter): Eventually check system tainted state and - // move this down in metrics library where it would be explicitly - // checked when asked to send stats. +static bool IsFeedbackAllowed() { + // Once crosbug.com/5814 is fixed, call the is opted in function + // here. return true; } -static void CheckUncleanShutdown() { - FilePath unclean_file_path(kUncleanShutdownFile); - if (!file_util::PathExists(unclean_file_path)) { - return; - } - s_system_log.LogWarning("Last shutdown was not clean"); - if (IsMetricsCollectionAllowed()) { - s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram), - kCrashKindKernel, - kCrashKindMax); - } - if (!file_util::Delete(unclean_file_path, false)) { - s_system_log.LogError("Failed to delete unclean shutdown file %s", - kUncleanShutdownFile); - } - - // Touch a file to notify the metrics daemon that a kernel crash has - // been detected so that it can log the time since the last kernel - // crash. - static const char kKernelCrashDetectedFile[] = "/tmp/kernel-crash-detected"; - FilePath crash_detected(kKernelCrashDetectedFile); - file_util::WriteFile(crash_detected, kEmpty, 0); +static bool TouchFile(const FilePath &file_path) { + return file_util::WriteFile(file_path, "", 0) == 0; } -static bool PrepareUncleanShutdownCheck() { - FilePath file_path(kUncleanShutdownFile); - file_util::CreateDirectory(file_path.DirName()); - return file_util::WriteFile(file_path, kEmpty, 0) == 0; +static void CountKernelCrash() { + s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram), + kCrashKindKernel, + kCrashKindMax); } -static void SignalCleanShutdown() { - s_system_log.LogInfo("Clean shutdown signalled"); - file_util::Delete(FilePath(kUncleanShutdownFile), false); +static void CountUncleanShutdown() { + s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram), + kCrashKindUncleanShutdown, + kCrashKindMax); } static void CountUserCrash() { - CHECK(IsMetricsCollectionAllowed()); s_metrics_lib.SendEnumToUMA(std::string(kCrashCounterHistogram), kCrashKindUser, kCrashKindMax); - + std::string command = StringPrintf( + "/usr/bin/dbus-send --type=signal --system / \"%s\"", + kUserCrashSignal); // Announce through D-Bus whenever a user crash happens. This is // used by the metrics daemon to log active use time between // crashes. @@ -93,42 +78,47 @@ static void CountUserCrash() { // using a dbus library directly. However, this should run // relatively rarely and longer term we may need to implement a // better way to do this that doesn't rely on D-Bus. - int status __attribute__((unused)) = - system("/usr/bin/dbus-send --type=signal --system / " - "org.chromium.CrashReporter.UserCrash"); + + int status __attribute__((unused)) = system(command.c_str()); } -int main(int argc, char *argv[]) { - google::ParseCommandLineFlags(&argc, &argv, true); - FilePath my_path(argv[0]); - file_util::AbsolutePath(&my_path); - s_metrics_lib.Init(); - s_system_log.Initialize(my_path.BaseName().value().c_str()); - UserCollector user_collector; - user_collector.Initialize(CountUserCrash, - my_path.value(), - IsMetricsCollectionAllowed, - &s_system_log, - true); // generate_diagnostics +static int Initialize(KernelCollector *kernel_collector, + UserCollector *user_collector, + UncleanShutdownCollector *unclean_shutdown_collector) { + CHECK(!FLAGS_clean_shutdown) << "Incompatible options"; - if (FLAGS_init) { - CHECK(!FLAGS_clean_shutdown) << "Incompatible options"; - user_collector.Enable(); - if (FLAGS_unclean_check) { - CheckUncleanShutdown(); - if (!PrepareUncleanShutdownCheck()) { - s_system_log.LogError("Unable to create shutdown check file"); - } + bool was_kernel_crash = false; + bool was_unclean_shutdown = false; + if (kernel_collector->IsEnabled()) { + was_kernel_crash = kernel_collector->Collect(); + } + + if (FLAGS_unclean_check) { + was_unclean_shutdown = unclean_shutdown_collector->Collect(); + } + + // Touch a file to notify the metrics daemon that a kernel + // crash has been detected so that it can log the time since + // the last kernel crash. + if (IsFeedbackAllowed()) { + if (was_kernel_crash) { + TouchFile(FilePath("/tmp/kernel-crash-detected")); + } else if (was_unclean_shutdown) { + // We only count an unclean shutdown if it did not come with + // an associated kernel crash. + TouchFile(FilePath("/tmp/unclean-shutdown-detected")); } - return 0; } - if (FLAGS_clean_shutdown) { - SignalCleanShutdown(); - user_collector.Disable(); - return 0; - } + // Must enable the unclean shutdown collector *after* collecting. + kernel_collector->Enable(); + unclean_shutdown_collector->Enable(); + user_collector->Enable(); + return 0; +} + +static int HandleUserCrash(UserCollector *user_collector) { // Handle a specific user space crash. CHECK(FLAGS_signal != -1) << "Signal must be set"; CHECK(FLAGS_pid != -1) << "PID must be set"; @@ -141,9 +131,45 @@ int main(int argc, char *argv[]) { } // Handle the crash, get the name of the process from procfs. - if (!user_collector.HandleCrash(FLAGS_signal, FLAGS_pid, NULL)) { + if (!user_collector->HandleCrash(FLAGS_signal, FLAGS_pid, NULL)) { return 1; } - return 0; } + + +int main(int argc, char *argv[]) { + google::ParseCommandLineFlags(&argc, &argv, true); + FilePath my_path(argv[0]); + file_util::AbsolutePath(&my_path); + s_metrics_lib.Init(); + s_system_log.Initialize(my_path.BaseName().value().c_str()); + KernelCollector kernel_collector; + kernel_collector.Initialize(CountKernelCrash, + IsFeedbackAllowed, + &s_system_log); + UserCollector user_collector; + user_collector.Initialize(CountUserCrash, + my_path.value(), + IsFeedbackAllowed, + &s_system_log, + true); // generate_diagnostics + UncleanShutdownCollector unclean_shutdown_collector; + unclean_shutdown_collector.Initialize(CountUncleanShutdown, + IsFeedbackAllowed, + &s_system_log); + + if (FLAGS_init) { + return Initialize(&kernel_collector, + &user_collector, + &unclean_shutdown_collector); + } + + if (FLAGS_clean_shutdown) { + unclean_shutdown_collector.Disable(); + user_collector.Disable(); + return 0; + } + + return HandleUserCrash(&user_collector); +} diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender index 99cc0def3..5ada4ef17 100644 --- a/crash_reporter/crash_sender +++ b/crash_reporter/crash_sender @@ -19,18 +19,19 @@ FIND="/usr/bin/find" # Send up to 8 crashes per day. MAX_CRASH_RATE=8 -# URL to send non-official build crashes to. -MINIDUMP_UPLOAD_STAGING_URL="http://clients2.google.com/cr/staging_report" - -# URL to send official build crashes to. -MINIDUMP_UPLOAD_PROD_URL="http://clients2.google.com/cr/report" - # File whose existence mocks crash sending. If empty we pretend the # crash sending was successful, otherwise unsuccessful. MOCK_CRASH_SENDING="/tmp/mock-crash-sending" # File whose existence causes crash sending to be delayed (for testing). -PAUSE_CRASH_SENDING="/tmp/pause-crash-sending" +# Must be stateful to enable testing kernel crashes. +PAUSE_CRASH_SENDING="/var/lib/crash_sender_paused" + +# URL to send non-official build crash reports to. +REPORT_UPLOAD_STAGING_URL="http://clients2.google.com/cr/staging_report" + +# URL to send official build crash reports to. +REPORT_UPLOAD_PROD_URL="http://clients2.google.com/cr/report" # File whose existence implies we're running and not to start again. RUN_FILE="/var/run/crash_sender.pid" @@ -118,31 +119,35 @@ check_rate() { } # Return if $1 is a .core file -is_core_file() { - local filename=$1 - [ "${filename##*.}" = "core" ] +get_kind() { + local kind="${1##*.}" + if [ "${kind}" = "dmp" ]; then + kind="minidump" + fi + echo ${kind} +} + +get_exec_name() { + local filename=$(basename "$1") + echo "${filename%%.*}" } send_crash() { + local report_path="$1" + local kind=$(get_kind "${report_path}") + local exec_name=$(get_exec_name "${report_path}") local sleep_time=$(generate_uniform_random $SECONDS_SEND_SPREAD) - local url="${MINIDUMP_UPLOAD_STAGING_URL}" + local url="${REPORT_UPLOAD_STAGING_URL}" if is_official; then - url="${MINIDUMP_UPLOAD_PROD_URL}" + url="${REPORT_UPLOAD_PROD_URL}" fi lecho "Sending crash:" lecho " Scheduled to send in ${sleep_time}s" - lecho " Minidump: ${minidump_path}" + lecho " Report: ${report_path} (${kind})" lecho " URL: ${url}" lecho " Product: ${CHROMEOS_PRODUCT}" lecho " Version: ${chromeos_version}" - if [ -s "${minidump_path}" ]; then - # We cannot tell much from the minidump without symbols, but we can tell - # at least what modules were loaded at the time of crash - local modules="$(/usr/bin/minidump_dump "${minidump_path}" 2>&- | \ - grep 'code_file' | sed -e 's/^.* = "//g;s/"//g' | \ - tr '\n' ' ')" - lecho " Mapped: ${modules}" - fi + lecho " Exec name: ${exec_name}" if [ -f "${MOCK_CRASH_SENDING}" ]; then local mock_in=$(cat "${MOCK_CRASH_SENDING}") if [ "${mock_in}" = "" ]; then @@ -166,9 +171,10 @@ send_crash() { curl "${url}" \ -F "prod=${CHROMEOS_PRODUCT}" \ -F "ver=${chromeos_version}" \ - -F "upload_file_minidump=@${minidump_path}" \ + -F "upload_file_${kind}=@${report_path}" \ + -F "exec_name=${exec_name}" \ -F "guid=<${CONSENT_ID}" -o "${report_id}" 2>"${curl_stderr}" - local curl_result=$? + curl_result=$? set -e if [ ${curl_result} -eq 0 ]; then @@ -194,28 +200,34 @@ send_crashes() { return fi for file in $(ls -1t "${dir}"); do - local minidump_path="${dir}/${file}" - lecho "Considering file ${minidump_path}" - if is_core_file "${minidump_path}"; then + local report_path="${dir}/${file}" + lecho "Considering file ${report_path}" + local kind=$(get_kind "${report_path}") + + if [ "${kind}" = "core" ]; then lecho "Ignoring core file." continue + elif [ "${kind}" != "minidump" ] && [ "${kind}" != "kcrash" ]; then + lecho "Unknown report kind: ${kind}. Removing report." + rm -f "${report_path}" + continue fi if ! check_rate; then - lecho "Sending ${minidump_path} would exceed rate. Leaving for later." + lecho "Sending ${report_path} would exceed rate. Leaving for later." return 0 fi local chromeos_version=$(get_version) if is_feedback_disabled; then lecho "Uploading is disabled. Removing crash." - rm "${minidump_path}" + rm "${report_path}" elif is_on_3g; then lecho "Not sending crash report while on 3G, saving for later." - elif send_crash ${minidump_path}; then + elif send_crash "${report_path}"; then # Send was successful, now remove - lecho "Successfully sent crash ${minidump_path} and removing" - rm "${minidump_path}" + lecho "Successfully sent crash ${report_path} and removing" + rm "${report_path}" else - lecho "Problem sending ${minidump_path}, not removing" + lecho "Problem sending ${report_path}, not removing" fi done } diff --git a/crash_reporter/kernel_collector.cc b/crash_reporter/kernel_collector.cc new file mode 100644 index 000000000..1a79c9d86 --- /dev/null +++ b/crash_reporter/kernel_collector.cc @@ -0,0 +1,118 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "crash-reporter/kernel_collector.h" + +#include "base/file_util.h" +#include "base/logging.h" +#include "base/string_util.h" +#include "crash-reporter/system_logging.h" + +static const char kKernelExecName[] = "kernel"; +static const char kPreservedDumpPath[] = "/sys/kernel/debug/preserved/kcrash"; +const pid_t kKernelPid = 0; +const uid_t kRootUid = 0; + +const char KernelCollector::kClearingSequence[] = " "; + +KernelCollector::KernelCollector() + : is_enabled_(false), + preserved_dump_path_(kPreservedDumpPath) { +} + +KernelCollector::~KernelCollector() { +} + +void KernelCollector::OverridePreservedDumpPath(const FilePath &file_path) { + preserved_dump_path_ = file_path; +} + +bool KernelCollector::LoadPreservedDump(std::string *contents) { + // clear contents since ReadFileToString actually appends to the string. + contents->clear(); + if (!file_util::ReadFileToString(preserved_dump_path_, contents)) { + logger_->LogError("Unable to read %s", + preserved_dump_path_.value().c_str()); + return false; + } + return true; +} + +bool KernelCollector::Enable() { + if (!file_util::PathExists(preserved_dump_path_)) { + logger_->LogWarning("Kernel does not support crash dumping"); + return false; + } + + // To enable crashes, we will eventually need to set + // the chnv bit in BIOS, but it does not yet work. + logger_->LogInfo("Enabling kernel crash handling"); + is_enabled_ = true; + return true; +} + +bool KernelCollector::ClearPreservedDump() { + // It is necessary to write at least one byte to the kcrash file for + // the log to actually be cleared. + if (file_util::WriteFile( + preserved_dump_path_, + kClearingSequence, + strlen(kClearingSequence)) != strlen(kClearingSequence)) { + logger_->LogError("Failed to clear kernel crash dump"); + return false; + } + logger_->LogInfo("Cleared kernel crash diagnostics"); + return true; +} + +FilePath KernelCollector::GetKernelCrashPath( + const FilePath &root_crash_path, + time_t timestamp) { + std::string dump_basename = + FormatDumpBasename(kKernelExecName, + timestamp, + kKernelPid); + return root_crash_path.Append( + StringPrintf("%s.kcrash", dump_basename.c_str())); +} + +bool KernelCollector::Collect() { + std::string kernel_dump; + FilePath root_crash_directory; + if (!LoadPreservedDump(&kernel_dump)) { + return false; + } + if (kernel_dump.empty()) { + return false; + } + if (is_feedback_allowed_function_()) { + count_crash_function_(); + + if (!GetCreatedCrashDirectoryByEuid(kRootUid, + &root_crash_directory)) { + return true; + } + + FilePath kernel_crash_path = GetKernelCrashPath(root_crash_directory, + time(NULL)); + if (file_util::WriteFile(kernel_crash_path, + kernel_dump.data(), + kernel_dump.length()) != + static_cast(kernel_dump.length())) { + logger_->LogInfo("Failed to write kernel dump to %s", + kernel_crash_path.value().c_str()); + return true; + } + + logger_->LogInfo("Collected kernel crash diagnostics into %s", + kernel_crash_path.value().c_str()); + } else { + logger_->LogInfo("Crash not saved since metrics disabled"); + } + if (!ClearPreservedDump()) { + return false; + } + + return true; +} diff --git a/crash_reporter/kernel_collector.h b/crash_reporter/kernel_collector.h new file mode 100644 index 000000000..3e2944245 --- /dev/null +++ b/crash_reporter/kernel_collector.h @@ -0,0 +1,54 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef _CRASH_REPORTER_KERNEL_COLLECTOR_H_ +#define _CRASH_REPORTER_KERNEL_COLLECTOR_H_ + +#include + +#include "base/file_path.h" +#include "crash-reporter/crash_collector.h" +#include "gtest/gtest_prod.h" // for FRIEND_TEST + +class FilePath; + +// Kernel crash collector. +class KernelCollector : public CrashCollector { + public: + KernelCollector(); + + virtual ~KernelCollector(); + + void OverridePreservedDumpPath(const FilePath &file_path); + + // Enable collection. + bool Enable(); + + // Returns true if the kernel collection currently enabled. + bool IsEnabled() { + return is_enabled_; + } + + // Collect any preserved kernel crash dump. Returns true if there was + // a dump (even if there were problems storing the dump), false otherwise. + bool Collect(); + + private: + friend class KernelCollectorTest; + FRIEND_TEST(KernelCollectorTest, ClearPreservedDump); + FRIEND_TEST(KernelCollectorTest, GetKernelCrashPath); + FRIEND_TEST(KernelCollectorTest, LoadPreservedDump); + FRIEND_TEST(KernelCollectorTest, CollectOK); + + bool LoadPreservedDump(std::string *contents); + bool ClearPreservedDump(); + FilePath GetKernelCrashPath(const FilePath &root_crash_path, + time_t timestamp); + + bool is_enabled_; + FilePath preserved_dump_path_; + static const char kClearingSequence[]; +}; + +#endif // _CRASH_REPORTER_KERNEL_COLLECTOR_H_ diff --git a/crash_reporter/kernel_collector_test.cc b/crash_reporter/kernel_collector_test.cc new file mode 100644 index 000000000..7188e2f15 --- /dev/null +++ b/crash_reporter/kernel_collector_test.cc @@ -0,0 +1,186 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "base/file_util.h" +#include "base/string_util.h" +#include "crash-reporter/kernel_collector.h" +#include "crash-reporter/system_logging_mock.h" +#include "gflags/gflags.h" +#include "gtest/gtest.h" + +static int s_crashes = 0; +static bool s_metrics = false; + +static const char kTestKCrash[] = "test/kcrash"; +static const char kTestCrashDirectory[] = "test/crash_directory"; + +void CountCrash() { + ++s_crashes; +} + +bool IsMetrics() { + return s_metrics; +} + +class KernelCollectorTest : public ::testing::Test { + void SetUp() { + s_crashes = 0; + s_metrics = true; + collector_.Initialize(CountCrash, + IsMetrics, + &logging_); + mkdir("test", 0777); + test_kcrash_ = FilePath(kTestKCrash); + collector_.OverridePreservedDumpPath(test_kcrash_); + unlink(kTestKCrash); + mkdir(kTestCrashDirectory, 0777); + } + protected: + void WriteStringToFile(const FilePath &file_path, + const char *data) { + ASSERT_EQ(strlen(data), + file_util::WriteFile(file_path, data, strlen(data))); + } + + void SetUpSuccessfulCollect(); + void CheckPreservedDumpClear(); + + SystemLoggingMock logging_; + KernelCollector collector_; + FilePath test_kcrash_; +}; + +TEST_F(KernelCollectorTest, LoadPreservedDump) { + ASSERT_FALSE(file_util::PathExists(test_kcrash_)); + std::string dump; + ASSERT_FALSE(collector_.LoadPreservedDump(&dump)); + WriteStringToFile(test_kcrash_, ""); + ASSERT_TRUE(collector_.LoadPreservedDump(&dump)); + ASSERT_EQ("", dump); + WriteStringToFile(test_kcrash_, "something"); + ASSERT_TRUE(collector_.LoadPreservedDump(&dump)); + ASSERT_EQ("something", dump); +} + +TEST_F(KernelCollectorTest, EnableMissingKernel) { + ASSERT_FALSE(collector_.Enable()); + ASSERT_FALSE(collector_.IsEnabled()); + ASSERT_EQ(std::string::npos, + logging_.log().find("Enabling kernel crash handling")); + ASSERT_NE(std::string::npos, + logging_.log().find("Kernel does not support crash dumping")); + ASSERT_EQ(s_crashes, 0); +} + +TEST_F(KernelCollectorTest, EnableOK) { + WriteStringToFile(test_kcrash_, ""); + ASSERT_TRUE(collector_.Enable()); + ASSERT_TRUE(collector_.IsEnabled()); + ASSERT_NE(std::string::npos, + logging_.log().find("Enabling kernel crash handling")); + ASSERT_EQ(s_crashes, 0); +} + +TEST_F(KernelCollectorTest, ClearPreservedDump) { + std::string dump; + ASSERT_FALSE(file_util::PathExists(test_kcrash_)); + WriteStringToFile(test_kcrash_, "something"); + ASSERT_TRUE(collector_.LoadPreservedDump(&dump)); + ASSERT_EQ("something", dump); + ASSERT_TRUE(collector_.ClearPreservedDump()); + ASSERT_TRUE(collector_.LoadPreservedDump(&dump)); + ASSERT_EQ(KernelCollector::kClearingSequence, dump); +} + +TEST_F(KernelCollectorTest, GetKernelCrashPath) { + FilePath root("/var/spool/crash"); + struct tm tm = {0}; + tm.tm_sec = 15; + tm.tm_min = 50; + tm.tm_hour = 13; + tm.tm_mday = 23; + tm.tm_mon = 4; + tm.tm_year = 110; + tm.tm_isdst = -1; + FilePath path = collector_.GetKernelCrashPath(root, mktime(&tm)); + ASSERT_EQ("/var/spool/crash/kernel.20100523.135015.0.kcrash", + path.value()); +} + +TEST_F(KernelCollectorTest, CollectPreservedFileMissing) { + ASSERT_FALSE(collector_.Collect()); + ASSERT_NE(logging_.log().find("Unable to read test/kcrash"), + std::string::npos); + ASSERT_EQ(0, s_crashes); +} + +TEST_F(KernelCollectorTest, CollectNoCrash) { + WriteStringToFile(test_kcrash_, ""); + ASSERT_FALSE(collector_.Collect()); + ASSERT_EQ(logging_.log().find("Collected kernel crash"), + std::string::npos); + ASSERT_EQ(0, s_crashes); +} + +TEST_F(KernelCollectorTest, CollectBadDirectory) { + WriteStringToFile(test_kcrash_, "something"); + ASSERT_TRUE(collector_.Collect()); + ASSERT_NE(logging_.log().find( + "Unable to create appropriate crash directory"), std::string::npos); + ASSERT_EQ(1, s_crashes); +} + +void KernelCollectorTest::SetUpSuccessfulCollect() { + collector_.ForceCrashDirectory(kTestCrashDirectory); + WriteStringToFile(test_kcrash_, "something"); + ASSERT_EQ(0, s_crashes); +} + +void KernelCollectorTest::CheckPreservedDumpClear() { + // Make sure the preserved dump is now clear. + std::string dump; + ASSERT_TRUE(collector_.LoadPreservedDump(&dump)); + ASSERT_EQ(KernelCollector::kClearingSequence, dump); +} + +TEST_F(KernelCollectorTest, CollectOptedOut) { + SetUpSuccessfulCollect(); + s_metrics = false; + ASSERT_TRUE(collector_.Collect()); + ASSERT_NE(std::string::npos, + logging_.log().find("Crash not saved since metrics disabled")); + ASSERT_EQ(0, s_crashes); + + CheckPreservedDumpClear(); +} + + +TEST_F(KernelCollectorTest, CollectOK) { + SetUpSuccessfulCollect(); + ASSERT_TRUE(collector_.Collect()); + ASSERT_EQ(1, s_crashes); + static const char kNamePrefix[] = "Collected kernel crash diagnostics into "; + size_t pos = logging_.log().find(kNamePrefix); + ASSERT_NE(std::string::npos, pos); + pos += strlen(kNamePrefix); + std::string filename = logging_.log().substr(pos, std::string::npos); + // Take the name up until \n + size_t end_pos = filename.find_first_of("\n"); + ASSERT_NE(std::string::npos, end_pos); + filename = filename.substr(0, end_pos); + ASSERT_EQ(0, filename.find(kTestCrashDirectory)); + ASSERT_TRUE(file_util::PathExists(FilePath(filename))); + std::string contents; + ASSERT_TRUE(file_util::ReadFileToString(FilePath(filename), &contents)); + ASSERT_EQ("something", contents); + + CheckPreservedDumpClear(); +} + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/crash_reporter/unclean_shutdown_collector.cc b/crash_reporter/unclean_shutdown_collector.cc new file mode 100644 index 000000000..bb2dd7a86 --- /dev/null +++ b/crash_reporter/unclean_shutdown_collector.cc @@ -0,0 +1,57 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "crash-reporter/unclean_shutdown_collector.h" + +#include "base/file_util.h" +#include "base/logging.h" +#include "crash-reporter/system_logging.h" + +static const char kUncleanShutdownFile[] = + "/var/lib/crash_reporter/pending_clean_shutdown"; + +UncleanShutdownCollector::UncleanShutdownCollector() + : unclean_shutdown_file_(kUncleanShutdownFile) { +} + +UncleanShutdownCollector::~UncleanShutdownCollector() { +} + +bool UncleanShutdownCollector::Enable() { + FilePath file_path(unclean_shutdown_file_); + file_util::CreateDirectory(file_path.DirName()); + if (file_util::WriteFile(file_path, "", 0) != 0) { + logger_->LogError("Unable to create shutdown check file"); + return false; + } + return true; +} + +bool UncleanShutdownCollector::DeleteUncleanShutdownFile() { + if (!file_util::Delete(FilePath(unclean_shutdown_file_), false)) { + logger_->LogError("Failed to delete unclean shutdown file %s", + unclean_shutdown_file_); + return false; + } + return true; +} + +bool UncleanShutdownCollector::Collect() { + FilePath unclean_file_path(unclean_shutdown_file_); + if (!file_util::PathExists(unclean_file_path)) { + return false; + } + logger_->LogWarning("Last shutdown was not clean"); + DeleteUncleanShutdownFile(); + + if (is_feedback_allowed_function_()) { + count_crash_function_(); + } + return true; +} + +bool UncleanShutdownCollector::Disable() { + logger_->LogInfo("Clean shutdown signalled"); + return DeleteUncleanShutdownFile(); +} diff --git a/crash_reporter/unclean_shutdown_collector.h b/crash_reporter/unclean_shutdown_collector.h new file mode 100644 index 000000000..e7d8cef95 --- /dev/null +++ b/crash_reporter/unclean_shutdown_collector.h @@ -0,0 +1,39 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef _CRASH_REPORTER_UNCLEAN_SHUTDOWN_COLLECTOR_H_ +#define _CRASH_REPORTER_UNCLEAN_SHUTDOWN_COLLECTOR_H_ + +#include + +#include "base/file_path.h" +#include "crash-reporter/crash_collector.h" +#include "gtest/gtest_prod.h" // for FRIEND_TEST + +// Unclean shutdown collector. +class UncleanShutdownCollector : public CrashCollector { + public: + UncleanShutdownCollector(); + virtual ~UncleanShutdownCollector(); + + // Enable collection - signal that a boot has started. + bool Enable(); + + // Collect if there is was an unclean shutdown. Returns true if + // there was, false otherwise. + bool Collect(); + + // Disable collection - signal that the system has been shutdown cleanly. + bool Disable(); + + private: + friend class UncleanShutdownCollectorTest; + FRIEND_TEST(UncleanShutdownCollectorTest, EnableCannotWrite); + + bool DeleteUncleanShutdownFile(); + + const char *unclean_shutdown_file_; +}; + +#endif // _CRASH_REPORTER_UNCLEAN_SHUTDOWN_COLLECTOR_H_ diff --git a/crash_reporter/unclean_shutdown_collector_test.cc b/crash_reporter/unclean_shutdown_collector_test.cc new file mode 100644 index 000000000..7be52af14 --- /dev/null +++ b/crash_reporter/unclean_shutdown_collector_test.cc @@ -0,0 +1,103 @@ +// Copyright (c) 2010 The Chromium OS Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include + +#include "base/file_util.h" +#include "base/string_util.h" +#include "crash-reporter/unclean_shutdown_collector.h" +#include "crash-reporter/system_logging_mock.h" +#include "gflags/gflags.h" +#include "gtest/gtest.h" + +static int s_crashes = 0; +static bool s_metrics = false; + +static const char kTestUnclean[] = "test/unclean"; + +void CountCrash() { + ++s_crashes; +} + +bool IsMetrics() { + return s_metrics; +} + +class UncleanShutdownCollectorTest : public ::testing::Test { + void SetUp() { + s_crashes = 0; + collector_.Initialize(CountCrash, + IsMetrics, + &logging_); + rmdir("test"); + test_unclean_ = FilePath(kTestUnclean); + collector_.unclean_shutdown_file_ = kTestUnclean; + file_util::Delete(test_unclean_, true); + } + protected: + void WriteStringToFile(const FilePath &file_path, + const char *data) { + ASSERT_EQ(strlen(data), + file_util::WriteFile(file_path, data, strlen(data))); + } + + SystemLoggingMock logging_; + UncleanShutdownCollector collector_; + FilePath test_unclean_; +}; + +TEST_F(UncleanShutdownCollectorTest, EnableWithoutParent) { + ASSERT_TRUE(collector_.Enable()); + ASSERT_TRUE(file_util::PathExists(test_unclean_)); +} + +TEST_F(UncleanShutdownCollectorTest, EnableWithParent) { + mkdir("test", 0777); + ASSERT_TRUE(collector_.Enable()); + ASSERT_TRUE(file_util::PathExists(test_unclean_)); +} + +TEST_F(UncleanShutdownCollectorTest, EnableCannotWrite) { + collector_.unclean_shutdown_file_ = "/bad/path"; + ASSERT_FALSE(collector_.Enable()); + ASSERT_NE(std::string::npos, + logging_.log().find("Unable to create shutdown check file")); +} + +TEST_F(UncleanShutdownCollectorTest, CollectTrue) { + ASSERT_TRUE(collector_.Enable()); + ASSERT_TRUE(file_util::PathExists(test_unclean_)); + ASSERT_TRUE(collector_.Collect()); + ASSERT_FALSE(file_util::PathExists(test_unclean_)); + ASSERT_NE(std::string::npos, + logging_.log().find("Last shutdown was not clean")); +} + +TEST_F(UncleanShutdownCollectorTest, CollectFalse) { + ASSERT_FALSE(collector_.Collect()); +} + +TEST_F(UncleanShutdownCollectorTest, Disable) { + ASSERT_TRUE(collector_.Enable()); + ASSERT_TRUE(file_util::PathExists(test_unclean_)); + ASSERT_TRUE(collector_.Disable()); + ASSERT_FALSE(file_util::PathExists(test_unclean_)); + ASSERT_FALSE(collector_.Collect()); +} + +TEST_F(UncleanShutdownCollectorTest, DisableWhenNotEnabled) { + ASSERT_TRUE(collector_.Disable()); +} + +TEST_F(UncleanShutdownCollectorTest, CantDisable) { + mkdir(kTestUnclean, 0700); + file_util::WriteFile(test_unclean_.Append("foo"), "", 0); + ASSERT_FALSE(collector_.Disable()); + rmdir(kTestUnclean); +} + +int main(int argc, char **argv) { + ::testing::InitGoogleTest(&argc, argv); + return RUN_ALL_TESTS(); +} diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc index 18ff39a09..e051a4ac3 100644 --- a/crash_reporter/user_collector.cc +++ b/crash_reporter/user_collector.cc @@ -2,6 +2,8 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include "crash-reporter/user_collector.h" + #include // For struct group. #include // For struct passwd. #include // For getpwuid_r and getgrnam_r. @@ -11,27 +13,14 @@ #include "base/file_util.h" #include "base/logging.h" #include "base/string_util.h" -#include "crash-reporter/user_collector.h" -#include "metrics/metrics_library.h" +#include "crash-reporter/system_logging.h" // This procfs file is used to cause kernel core file writing to // instead pipe the core file into a user space process. See // core(5) man page. static const char kCorePatternFile[] = "/proc/sys/kernel/core_pattern"; static const char kCoreToMinidumpConverterPath[] = "/usr/bin/core2md"; -static const char kDefaultUserName[] = "chronos"; static const char kLeaveCoreFile[] = "/root/.leave_core"; -static const char kSystemCrashPath[] = "/var/spool/crash"; -static const char kUserCrashPath[] = "/home/chronos/user/crash"; - -// Directory mode of the user crash spool directory. -static const mode_t kUserCrashPathMode = 0755; - -// Directory mode of the system crash spool directory. -static const mode_t kSystemCrashPathMode = 01755; - -static const uid_t kRootOwner = 0; -static const uid_t kRootGroup = 0; const char *UserCollector::kUserId = "Uid:\t"; const char *UserCollector::kGroupId = "Gid:\t"; @@ -39,10 +28,7 @@ const char *UserCollector::kGroupId = "Gid:\t"; UserCollector::UserCollector() : generate_diagnostics_(false), core_pattern_file_(kCorePatternFile), - count_crash_function_(NULL), - initialized_(false), - is_feedback_allowed_function_(NULL), - logger_(NULL) { + initialized_(false) { } void UserCollector::Initialize( @@ -51,14 +37,10 @@ void UserCollector::Initialize( UserCollector::IsFeedbackAllowedFunction is_feedback_allowed_function, SystemLogging *logger, bool generate_diagnostics) { - CHECK(count_crash_function != NULL); - CHECK(is_feedback_allowed_function != NULL); - CHECK(logger != NULL); - - count_crash_function_ = count_crash_function; + CrashCollector::Initialize(count_crash_function, + is_feedback_allowed_function, + logger); our_path_ = our_path; - is_feedback_allowed_function_ = is_feedback_allowed_function; - logger_ = logger; initialized_ = true; generate_diagnostics_ = generate_diagnostics; } @@ -76,7 +58,8 @@ std::string UserCollector::GetPattern(bool enabled) const { bool UserCollector::SetUpInternal(bool enabled) { CHECK(initialized_); - logger_->LogInfo("%s crash handling", enabled ? "Enabling" : "Disabling"); + logger_->LogInfo("%s user crash handling", + enabled ? "Enabling" : "Disabling"); std::string pattern = GetPattern(enabled); if (file_util::WriteFile(FilePath(core_pattern_file_), pattern.c_str(), @@ -161,24 +144,6 @@ bool UserCollector::GetIdFromStatus(const char *prefix, return true; } -bool UserCollector::GetUserInfoFromName(const std::string &name, - uid_t *uid, - gid_t *gid) { - char storage[256]; - struct passwd passwd_storage; - struct passwd *passwd_result = NULL; - - if (getpwnam_r(name.c_str(), &passwd_storage, storage, sizeof(storage), - &passwd_result) != 0 || passwd_result == NULL) { - logger_->LogError("Cannot find user named %s", name.c_str()); - return false; - } - - *uid = passwd_result->pw_uid; - *gid = passwd_result->pw_gid; - return true; -} - bool UserCollector::CopyOffProcFiles(pid_t pid, const FilePath &container_dir) { if (!file_util::CreateDirectory(container_dir)) { @@ -208,26 +173,6 @@ bool UserCollector::CopyOffProcFiles(pid_t pid, return true; } -FilePath UserCollector::GetCrashDirectoryInfo( - uid_t process_euid, - uid_t default_user_id, - gid_t default_user_group, - mode_t *mode, - uid_t *directory_owner, - gid_t *directory_group) { - if (process_euid == default_user_id) { - *mode = kUserCrashPathMode; - *directory_owner = default_user_id; - *directory_group = default_user_group; - return FilePath(kUserCrashPath); - } else { - *mode = kSystemCrashPathMode; - *directory_owner = kRootOwner; - *directory_group = kRootGroup; - return FilePath(kSystemCrashPath); - } -} - bool UserCollector::GetCreatedCrashDirectory(pid_t pid, FilePath *crash_file_path) { FilePath process_path = GetProcessPath(pid); @@ -242,64 +187,8 @@ bool UserCollector::GetCreatedCrashDirectory(pid_t pid, logger_->LogError("Could not find euid in status file"); return false; } - uid_t default_user_id; - gid_t default_user_group; - if (!GetUserInfoFromName(kDefaultUserName, - &default_user_id, - &default_user_group)) { - logger_->LogError("Could not find default user info"); - return false; - } - mode_t directory_mode; - uid_t directory_owner; - gid_t directory_group; - *crash_file_path = - GetCrashDirectoryInfo(process_euid, - default_user_id, - default_user_group, - &directory_mode, - &directory_owner, - &directory_group); - - - if (!file_util::PathExists(*crash_file_path)) { - // Create the spool directory with the appropriate mode (regardless of - // umask) and ownership. - mode_t old_mask = umask(0); - if (mkdir(crash_file_path->value().c_str(), directory_mode) < 0 || - chown(crash_file_path->value().c_str(), - directory_owner, - directory_group) < 0) { - logger_->LogError("Unable to create appropriate crash directory"); - return false; - } - umask(old_mask); - } - - if (!file_util::PathExists(*crash_file_path)) { - logger_->LogError("Unable to create crash directory %s", - crash_file_path->value().c_str()); - return false; - } - - - return true; -} - -std::string UserCollector::FormatDumpBasename(const std::string &exec_name, - time_t timestamp, - pid_t pid) { - struct tm tm; - localtime_r(×tamp, &tm); - return StringPrintf("%s.%04d%02d%02d.%02d%02d%02d.%d", - exec_name.c_str(), - tm.tm_year + 1900, - tm.tm_mon + 1, - tm.tm_mday, - tm.tm_hour, - tm.tm_min, - tm.tm_sec, - pid); + return GetCreatedCrashDirectoryByEuid(process_euid, + crash_file_path); } bool UserCollector::CopyStdinToCoreFile(const FilePath &core_path) { @@ -360,13 +249,13 @@ bool UserCollector::GenerateDiagnostics(pid_t pid, return false; } - FilePath spool_path; - if (!GetCreatedCrashDirectory(pid, &spool_path)) { + FilePath crash_path; + if (!GetCreatedCrashDirectory(pid, &crash_path)) { file_util::Delete(container_dir, true); return false; } std::string dump_basename = FormatDumpBasename(exec_name, time(NULL), pid); - FilePath core_path = spool_path.Append( + FilePath core_path = crash_path.Append( StringPrintf("%s.core", dump_basename.c_str())); if (!CopyStdinToCoreFile(core_path)) { @@ -374,7 +263,7 @@ bool UserCollector::GenerateDiagnostics(pid_t pid, return false; } - FilePath minidump_path = spool_path.Append( + FilePath minidump_path = crash_path.Append( StringPrintf("%s.dmp", dump_basename.c_str())); bool conversion_result = true; @@ -414,10 +303,10 @@ bool UserCollector::HandleCrash(int signal, int pid, const char *force_exec) { if (is_feedback_allowed_function_()) { count_crash_function_(); - } - if (generate_diagnostics_) { - return GenerateDiagnostics(pid, exec); + if (generate_diagnostics_) { + return GenerateDiagnostics(pid, exec); + } } return true; } diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h index dc633bba4..484445fcb 100644 --- a/crash_reporter/user_collector.h +++ b/crash_reporter/user_collector.h @@ -7,17 +7,15 @@ #include -#include "crash-reporter/system_logging.h" +#include "crash-reporter/crash_collector.h" #include "gtest/gtest_prod.h" // for FRIEND_TEST class FilePath; +class SystemLogging; // User crash collector. -class UserCollector { +class UserCollector : public CrashCollector { public: - typedef void (*CountCrashFunction)(); - typedef bool (*IsFeedbackAllowedFunction)(); - UserCollector(); // Initialize the user crash collector for detection of crashes, @@ -53,8 +51,6 @@ class UserCollector { FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPath); FRIEND_TEST(UserCollectorTest, CopyOffProcFilesBadPid); FRIEND_TEST(UserCollectorTest, CopyOffProcFilesOK); - FRIEND_TEST(UserCollectorTest, FormatDumpBasename); - FRIEND_TEST(UserCollectorTest, GetCrashDirectoryInfo); FRIEND_TEST(UserCollectorTest, GetIdFromStatus); FRIEND_TEST(UserCollectorTest, GetProcessPath); FRIEND_TEST(UserCollectorTest, GetSymlinkTarget); @@ -82,25 +78,13 @@ class UserCollector { IdKind kind, const std::string &status_contents, int *id); - bool GetUserInfoFromName(const std::string &name, - uid_t *uid, - gid_t *gid); bool CopyOffProcFiles(pid_t pid, const FilePath &process_map); - FilePath GetCrashDirectoryInfo(uid_t process_euid, - uid_t default_user_id, - gid_t default_user_group, - mode_t *mode, - uid_t *directory_owner, - gid_t *directory_group); // Determines the crash directory for given pid based on pid's owner, // and creates the directory if necessary with appropriate permissions. // Returns true whether or not directory needed to be created, false on // any failure. bool GetCreatedCrashDirectory(pid_t pid, FilePath *crash_file_path); - std::string FormatDumpBasename(const std::string &exec_name, - time_t timestamp, - pid_t pid); bool CopyStdinToCoreFile(const FilePath &core_path); bool ConvertCoreToMinidump(const FilePath &core_path, const FilePath &procfs_directory, @@ -110,11 +94,8 @@ class UserCollector { bool generate_diagnostics_; std::string core_pattern_file_; - CountCrashFunction count_crash_function_; std::string our_path_; bool initialized_; - IsFeedbackAllowedFunction is_feedback_allowed_function_; - SystemLogging *logger_; static const char *kUserId; static const char *kGroupId; diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc index 335821c74..3390caab9 100644 --- a/crash_reporter/user_collector_test.cc +++ b/crash_reporter/user_collector_test.cc @@ -10,8 +10,8 @@ #include "gflags/gflags.h" #include "gtest/gtest.h" -int s_crashes = 0; -bool s_metrics = false; +static int s_crashes = 0; +static bool s_metrics = false; static const char kFilePath[] = "/my/path"; @@ -50,14 +50,16 @@ TEST_F(UserCollectorTest, EnableOK) { &contents)); ASSERT_EQ("|/my/path --signal=%s --pid=%p", contents); ASSERT_EQ(s_crashes, 0); - ASSERT_NE(logging_.log().find("Enabling crash handling"), std::string::npos); + ASSERT_NE(logging_.log().find("Enabling user crash handling"), + std::string::npos); } TEST_F(UserCollectorTest, EnableNoFileAccess) { collector_.set_core_pattern_file("/does_not_exist"); ASSERT_FALSE(collector_.Enable()); ASSERT_EQ(s_crashes, 0); - ASSERT_NE(logging_.log().find("Enabling crash handling"), std::string::npos); + ASSERT_NE(logging_.log().find("Enabling user crash handling"), + std::string::npos); ASSERT_NE(logging_.log().find("Unable to write /does_not_exist"), std::string::npos); } @@ -69,7 +71,7 @@ TEST_F(UserCollectorTest, DisableOK) { &contents)); ASSERT_EQ("core", contents); ASSERT_EQ(s_crashes, 0); - ASSERT_NE(logging_.log().find("Disabling crash handling"), + ASSERT_NE(logging_.log().find("Disabling user crash handling"), std::string::npos); } @@ -77,7 +79,8 @@ TEST_F(UserCollectorTest, DisableNoFileAccess) { collector_.set_core_pattern_file("/does_not_exist"); ASSERT_FALSE(collector_.Disable()); ASSERT_EQ(s_crashes, 0); - ASSERT_NE(logging_.log().find("Disabling crash handling"), std::string::npos); + ASSERT_NE(logging_.log().find("Disabling user crash handling"), + std::string::npos); ASSERT_NE(logging_.log().find("Unable to write /does_not_exist"), std::string::npos); } @@ -206,54 +209,6 @@ TEST_F(UserCollectorTest, GetUserInfoFromName) { EXPECT_EQ(0, gid); } -TEST_F(UserCollectorTest, GetCrashDirectoryInfo) { - FilePath path; - const int kRootUid = 0; - const int kRootGid = 0; - const int kNtpUid = 5; - const int kChronosUid = 1000; - const int kChronosGid = 1001; - const mode_t kExpectedSystemMode = 01755; - const mode_t kExpectedUserMode = 0755; - - mode_t directory_mode; - uid_t directory_owner; - gid_t directory_group; - - path = collector_.GetCrashDirectoryInfo(kRootUid, - kChronosUid, - kChronosGid, - &directory_mode, - &directory_owner, - &directory_group); - EXPECT_EQ("/var/spool/crash", path.value()); - EXPECT_EQ(kExpectedSystemMode, directory_mode); - EXPECT_EQ(kRootUid, directory_owner); - EXPECT_EQ(kRootGid, directory_group); - - path = collector_.GetCrashDirectoryInfo(kNtpUid, - kChronosUid, - kChronosGid, - &directory_mode, - &directory_owner, - &directory_group); - EXPECT_EQ("/var/spool/crash", path.value()); - EXPECT_EQ(kExpectedSystemMode, directory_mode); - EXPECT_EQ(kRootUid, directory_owner); - EXPECT_EQ(kRootGid, directory_group); - - path = collector_.GetCrashDirectoryInfo(kChronosUid, - kChronosUid, - kChronosGid, - &directory_mode, - &directory_owner, - &directory_group); - EXPECT_EQ("/home/chronos/user/crash", path.value()); - EXPECT_EQ(kExpectedUserMode, directory_mode); - EXPECT_EQ(kChronosUid, directory_owner); - EXPECT_EQ(kChronosGid, directory_group); -} - TEST_F(UserCollectorTest, CopyOffProcFilesBadPath) { // Try a path that is not writable. ASSERT_FALSE(collector_.CopyOffProcFiles(pid_, FilePath("/bad/path"))); @@ -295,20 +250,6 @@ TEST_F(UserCollectorTest, CopyOffProcFilesOK) { } } -TEST_F(UserCollectorTest, FormatDumpBasename) { - struct tm tm = {0}; - tm.tm_sec = 15; - tm.tm_min = 50; - tm.tm_hour = 13; - tm.tm_mday = 23; - tm.tm_mon = 4; - tm.tm_year = 110; - tm.tm_isdst = -1; - std::string basename = - collector_.FormatDumpBasename("foo", mktime(&tm), 100); - ASSERT_EQ("foo.20100523.135015.100", basename); -} - int main(int argc, char **argv) { ::testing::InitGoogleTest(&argc, argv); return RUN_ALL_TESTS();