Merge "crash_reporter: Fix product_id in kernel crash reports" (26815151) · Commits · e / os / android_system_core

crash_reporter/crash_collector.cc

+42 −1

Original line number	Diff line number	Diff line
		@@ -36,6 +36,7 @@
		#include <base/strings/string_util.h>
		#include <base/strings/stringprintf.h>
		#include <brillo/key_value_store.h>
		#include <brillo/osrelease_reader.h>
		#include <brillo/process.h>

		namespace {
		@@ -52,6 +53,11 @@ const char kSystemCrashPath[] = "/data/misc/crash_reporter/crash";
		const char kUploadVarPrefix[] = "upload_var_";
		const char kUploadFilePrefix[] = "upload_file_";

		// Product information keys in the /etc/os-release.d folder.
		static const char kBdkVersionKey[] = "bdk_version";
		static const char kProductIDKey[] = "product_id";
		static const char kProductVersionKey[] = "product_version";

		// Normally this path is not used. Unfortunately, there are a few edge cases
		// where we need this. Any process that runs as kDefaultUserName that crashes
		// is consider a "user crash". That includes the initial Chrome browser that
		@@ -384,14 +390,49 @@ void CrashCollector::WriteCrashMetaData(const FilePath &meta_path,
		const std::string &payload_path) {
		int64_t payload_size = -1;
		base::GetFileSize(FilePath(payload_path), &payload_size);

		brillo::OsReleaseReader reader;
		if (!forced_osreleased_directory_.empty()) {
		reader.LoadTestingOnly(forced_osreleased_directory_);
		} else {
		reader.Load();
		}
		std::string bdk_version = "undefined";
		std::string product_id = "undefined";
		std::string product_version = "undefined";

		if (!reader.GetString(kBdkVersionKey, &bdk_version)) {
		LOG(ERROR) << "Could not read " << kBdkVersionKey
		<< " from /etc/os-release.d/";
		}

		if (!reader.GetString(kProductIDKey, &product_id)) {
		LOG(ERROR) << "Could not read " << kProductIDKey
		<< " from /etc/os-release.d/";
		}

		if (!reader.GetString(kProductVersionKey, &product_version)) {
		LOG(ERROR) << "Could not read " << kProductVersionKey
		<< " from /etc/os-release.d/";
		}

		std::string meta_data = StringPrintf("%sexec_name=%s\n"
		"payload=%s\n"
		"payload_size=%" PRId64 "\n"
		"%s=%s\n"
		"%s=%s\n"
		"%s=%s\n"
		"done=1\n",
		extra_metadata_.c_str(),
		exec_name.c_str(),
		payload_path.c_str(),
		payload_size);
		payload_size,
		kBdkVersionKey,
		bdk_version.c_str(),
		kProductIDKey,
		product_id.c_str(),
		kProductVersionKey,
		product_version.c_str());
		// We must use WriteNewFile instead of base::WriteFile as we
		// do not want to write with root access to a symlink that an attacker
		// might have created.

crash_reporter/crash_collector.h

+7 −0

Original line number	Diff line number	Diff line
		@@ -84,6 +84,12 @@ class CrashCollector {
		forced_crash_directory_ = forced_directory;
		}

		// For testing, set the root directory to read etc/os-release.d properties
		// from.
		void ForceOsReleaseDDirectory(const base::FilePath &forced_directory) {
		forced_osreleased_directory_ = forced_directory;
		}

		base::FilePath GetCrashDirectoryInfo(mode_t *mode,
		uid_t *directory_owner,
		gid_t *directory_group);
		@@ -158,6 +164,7 @@ class CrashCollector {
		IsFeedbackAllowedFunction is_feedback_allowed_function_;
		std::string extra_metadata_;
		base::FilePath forced_crash_directory_;
		base::FilePath forced_osreleased_directory_;
		base::FilePath log_config_path_;

		private:

crash_reporter/crash_collector_test.cc

+20 −0

Original line number	Diff line number	Diff line
		@@ -182,9 +182,26 @@ TEST_F(CrashCollectorTest, MetaData) {
		const char kMetaFileBasename[] = "generated.meta";
		FilePath meta_file = test_dir_.path().Append(kMetaFileBasename);
		FilePath payload_file = test_dir_.path().Append("payload-file");
		FilePath osreleased_directory =
		test_dir_.path().Append("etc").Append("os-release.d");
		ASSERT_TRUE(base::CreateDirectory(osreleased_directory));
		collector_.ForceOsReleaseDDirectory(test_dir_.path());

		std::string contents;
		const char kPayload[] = "foo";
		ASSERT_TRUE(base::WriteFile(payload_file, kPayload, strlen(kPayload)));
		const char kBdkVersion[] = "1";
		ASSERT_TRUE(base::WriteFile(osreleased_directory.Append("bdk_version"),
		kBdkVersion,
		strlen(kBdkVersion)));
		const char kProductId[] = "baz";
		ASSERT_TRUE(base::WriteFile(osreleased_directory.Append("product_id"),
		kProductId,
		strlen(kProductId)));
		const char kProductVersion[] = "1.2.3.4";
		ASSERT_TRUE(base::WriteFile(osreleased_directory.Append("product_version"),
		kProductVersion,
		strlen(kProductVersion)));
		collector_.AddCrashMetaData("foo", "bar");
		collector_.WriteCrashMetaData(meta_file, "kernel", payload_file.value());
		EXPECT_TRUE(base::ReadFileToString(meta_file, &contents));
		@@ -193,6 +210,9 @@ TEST_F(CrashCollectorTest, MetaData) {
		"exec_name=kernel\n"
		"payload=%s\n"
		"payload_size=3\n"
		"bdk_version=1\n"
		"product_id=baz\n"
		"product_version=1.2.3.4\n"
		"done=1\n",
		test_dir_.path().Append("payload-file").value().c_str());
		EXPECT_EQ(kExpectedMeta, contents);

crash_reporter/user_collector.cc

+0 −29

Original line number	Diff line number	Diff line
		@@ -37,7 +37,6 @@
		#include <base/strings/string_split.h>
		#include <base/strings/string_util.h>
		#include <base/strings/stringprintf.h>
		#include <brillo/osrelease_reader.h>
		#include <brillo/process.h>
		#include <brillo/syslog_logging.h>
		#include <cutils/properties.h>
		@@ -59,11 +58,6 @@ static const gid_t kUnknownGid = -1;
		const char *UserCollector::kUserId = "Uid:\t";
		const char *UserCollector::kGroupId = "Gid:\t";

		// Product information keys in the /etc/os-release.d folder.
		static const char kBdkVersionKey[] = "bdk_version";
		static const char kProductIDKey[] = "product_id";
		static const char kProductVersionKey[] = "product_version";


		using base::FilePath;
		using base::StringPrintf;
		@@ -505,29 +499,6 @@ UserCollector::ErrorType UserCollector::ConvertAndEnqueueCrash(
		if (GetLogContents(FilePath(log_config_path_), exec, log_path))
		AddCrashMetaData("log", log_path.value());

		brillo::OsReleaseReader reader;
		reader.Load();
		std::string value = "undefined";
		if (!reader.GetString(kBdkVersionKey, &value)) {
		LOG(ERROR) << "Could not read " << kBdkVersionKey
		<< " from /etc/os-release.d/";
		}
		AddCrashMetaData(kBdkVersionKey, value);

		value = "undefined";
		if (!reader.GetString(kProductIDKey, &value)) {
		LOG(ERROR) << "Could not read " << kProductIDKey
		<< " from /etc/os-release.d/";
		}
		AddCrashMetaData(kProductIDKey, value);

		value = "undefined";
		if (!reader.GetString(kProductVersionKey, &value)) {
		LOG(ERROR) << "Could not read " << kProductVersionKey
		<< " from /etc/os-release.d/";
		}
		AddCrashMetaData(kProductVersionKey, value);

		ErrorType error_type =
		ConvertCoreToMinidump(pid, container_dir, core_path, minidump_path);
		if (error_type != kErrorNone) {