add support for multiprofile
Update crash_sender to scan /home/chronos/u-*/crash/ paths since it's
a shell script and there's no easy way to get the right info via shell
commands. We want to scan all paths in case of different ordering.
For crash_collector, update it to use SessionManager's dbus call to
query the active profiles. We select the first one and use that to
process crashes. This should be fine.
We also need to handle the edge case where no user is logged in (yet a
crash occurs with a program running as chronos uid; e.g. the login).
In the past, we just wrote to /home/chronos/user/crash/ even when there
wasn't a user home dir mounted there. With this change, we formalize
(and document) things a bit more by moving to /home/chronos/crash/. We
want this behavior rather than re-using the system path as our tests
specifically verify system vs user crashes.
BUG=chromium:221778
TEST=`cros_run_unit_tests --board=x86-alex -p crash-reporter` passes
TEST=logging_UserCrash autotest passes in a vm
CQ-DEPEND=CL:56112
CQ-DEPEND=CL:57067
Change-Id: If9f2ffd0740533537718b2a5e7d215200f5a41d6
Reviewed-on: https://gerrit.chromium.org/gerrit/55600
Reviewed-by: Mike Frysinger <vapier@chromium.org>
Tested-by: Mike Frysinger <vapier@chromium.org>
Commit-Queue: Mike Frysinger <vapier@chromium.org>
diff --git a/crash_reporter/Makefile b/crash_reporter/Makefile
index c784e6e..4f716fa 100644
--- a/crash_reporter/Makefile
+++ b/crash_reporter/Makefile
@@ -18,17 +18,23 @@
unclean_shutdown_collector.o \
user_collector.o
-PC_DEPS = glib-2.0 libpcrecpp libchrome-$(BASE_VER) libchromeos-$(BASE_VER)
+PC_DEPS = glib-2.0 gobject-2.0 libpcrecpp libchrome-$(BASE_VER) \
+ libchromeos-$(BASE_VER)
PC_CFLAGS := $(shell $(PKG_CONFIG) --cflags $(PC_DEPS))
PC_LIBS := $(shell $(PKG_CONFIG) --libs $(PC_DEPS))
+DBUS_PC_DEPS = dbus-1 dbus-glib-1
+DBUS_PC_CFLAGS := $(shell $(PKG_CONFIG) --cflags $(DBUS_PC_DEPS))
+DBUS_PC_LIBS := $(shell $(PKG_CONFIG) --libs $(DBUS_PC_DEPS))
+
CPPFLAGS += -I$(SRC)/.. -I$(SYSROOT)/usr/include/google-breakpad $(PC_CFLAGS)
LDLIBS = -lgflags $(PC_LIBS)
# -- Executable crash_reporter --
-CXX_BINARY(crash_reporter): LDLIBS += -lmetrics
+CXX_BINARY(crash_reporter): CPPFLAGS += $(DBUS_PC_CFLAGS)
+CXX_BINARY(crash_reporter): LDLIBS += -lmetrics $(DBUS_PC_LIBS)
CXX_BINARY(crash_reporter): crash_reporter.o $(CRASH_OBJS)
clean: CLEAN(crash_reporter)
all: CXX_BINARY(crash_reporter)
@@ -36,11 +42,8 @@
# -- Executable list_proxies --
-LIST_PROXIES_PKGS = dbus-1 dbus-glib-1
-LIST_PROXIES_CFLAGS := $(shell $(PKG_CONFIG) --cflags $(LIST_PROXIES_PKGS))
-LIST_PROXIES_LIBS := $(shell $(PKG_CONFIG) --libs $(LIST_PROXIES_PKGS))
-CXX_BINARY(list_proxies): CPPFLAGS += $(LIST_PROXIES_CFLAGS)
-CXX_BINARY(list_proxies): LDLIBS += $(LIST_PROXIES_LIBS)
+CXX_BINARY(list_proxies): CPPFLAGS += $(DBUS_PC_CFLAGS)
+CXX_BINARY(list_proxies): LDLIBS += $(DBUS_PC_LIBS)
CXX_BINARY(list_proxies): list_proxies.o
clean: CLEAN(list_proxies)
all: CXX_BINARY(list_proxies)
@@ -60,6 +63,8 @@
# -- Unit Tests --
+UNITTEST_LIBS := $(shell gmock-config --libs) $(shell gtest-config --libs)
+
# Uncomment these to just run specific test(s).
#TEST_BINS += chrome_collector_test
#TEST_BINS += crash_collector_test
@@ -74,7 +79,7 @@
# be for "CXX_BINARY(<test name>)". For example, you could add options with:
# CXX_BINARY(crash_collector_test): CXXFLAGS += -some-other-option
define TEST_RULES_template
-CXX_BINARY($(1)): LDLIBS += -lgtest -lgmock
+CXX_BINARY($(1)): LDLIBS += $(UNITTEST_LIBS) $(DBUS_PC_LIBS)
CXX_BINARY($(1)): $(1).o $(CRASH_OBJS)
clean: CLEAN($(1))
tests: TEST(CXX_BINARY($(1)))
diff --git a/crash_reporter/crash_collector.cc b/crash_reporter/crash_collector.cc
index 85c0933..55fdd57 100644
--- a/crash_reporter/crash_collector.cc
+++ b/crash_reporter/crash_collector.cc
@@ -16,12 +16,18 @@
#include <set>
#include <vector>
+#include <dbus/dbus-glib-lowlevel.h>
+#include <glib.h>
+
#include "base/file_util.h"
#include "base/logging.h"
#include "base/posix/eintr_wrapper.h"
#include "base/string_split.h"
#include "base/string_util.h"
#include "base/stringprintf.h"
+#include "chromeos/cryptohome.h"
+#include "chromeos/dbus/dbus.h"
+#include "chromeos/dbus/service_constants.h"
#include "chromeos/process.h"
static const char kCollectChromeFile[] =
@@ -33,7 +39,17 @@
static const char kLsbRelease[] = "/etc/lsb-release";
static const char kShellPath[] = "/bin/sh";
static const char kSystemCrashPath[] = "/var/spool/crash";
-static const char kUserCrashPath[] = "/home/chronos/user/crash";
+// 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
+// runs the login screen. If that blows up, there is no logged in user yet,
+// so there is no per-user dir for us to stash things in. Instead we fallback
+// to this path as it is at least encrypted on a per-system basis.
+//
+// This also comes up when running autotests. The GUI is sitting at the login
+// screen while tests are sshing in, changing users, and triggering crashes as
+// the user (purposefully).
+static const char kFallbackUserCrashPath[] = "/home/chronos/crash";
// Directory mode of the user crash spool directory.
static const mode_t kUserCrashPathMode = 0755;
@@ -123,6 +139,72 @@
extension.c_str()));
}
+namespace {
+
+const char *GetGErrorMessage(const GError *error) {
+ if (!error)
+ return "Unknown error.";
+ return error->message;
+}
+
+}
+
+GHashTable *CrashCollector::GetActiveUserSessions(void) {
+ GHashTable *active_sessions = NULL;
+
+ chromeos::dbus::BusConnection dbus = chromeos::dbus::GetSystemBusConnection();
+ if (!dbus.HasConnection()) {
+ LOG(ERROR) << "Error connecting to system D-Bus";
+ return active_sessions;
+ }
+ chromeos::dbus::Proxy proxy(dbus,
+ login_manager::kSessionManagerServiceName,
+ login_manager::kSessionManagerServicePath,
+ login_manager::kSessionManagerInterface);
+ if (!proxy) {
+ LOG(ERROR) << "Error creating D-Bus proxy to interface "
+ << "'" << login_manager::kSessionManagerServiceName << "'";
+ return active_sessions;
+ }
+
+ // Request all the active sessions.
+ GError *gerror = NULL;
+ if (!dbus_g_proxy_call(proxy.gproxy(),
+ login_manager::kSessionManagerRetrieveActiveSessions,
+ &gerror, G_TYPE_INVALID,
+ DBUS_TYPE_G_STRING_STRING_HASHTABLE, &active_sessions,
+ G_TYPE_INVALID)) {
+ LOG(ERROR) << "Error performing D-Bus proxy call "
+ << "'"
+ << login_manager::kSessionManagerRetrieveActiveSessions << "'"
+ << ": " << GetGErrorMessage(gerror);
+ return active_sessions;
+ }
+
+ return active_sessions;
+}
+
+FilePath CrashCollector::GetUserCrashPath(void) {
+ // In this multiprofile world, there is no one-specific user dir anymore.
+ // Ask the session manager for the active ones, then just run with the
+ // first result we get back.
+ FilePath user_path = FilePath(kFallbackUserCrashPath);
+ GHashTable *active_sessions = GetActiveUserSessions();
+ if (!active_sessions)
+ return user_path;
+
+ GList *list = g_hash_table_get_values(active_sessions);
+ if (list) {
+ const char *salted_path = static_cast<const char *>(list->data);
+ user_path = chromeos::cryptohome::home::GetHashedUserPath(salted_path);
+ g_list_free(list);
+ }
+
+ g_hash_table_destroy(active_sessions);
+
+ return user_path;
+}
+
FilePath CrashCollector::GetCrashDirectoryInfo(
uid_t process_euid,
uid_t default_user_id,
@@ -140,7 +222,7 @@
*mode = kUserCrashPathMode;
*directory_owner = default_user_id;
*directory_group = default_user_group;
- return FilePath(kUserCrashPath);
+ return GetUserCrashPath();
} else {
*mode = kSystemCrashPathMode;
*directory_owner = kRootOwner;
diff --git a/crash_reporter/crash_collector.h b/crash_reporter/crash_collector.h
index 949552a..4cbe1b9 100644
--- a/crash_reporter/crash_collector.h
+++ b/crash_reporter/crash_collector.h
@@ -10,6 +10,8 @@
#include <map>
#include <string>
+#include <glib.h>
+
#include "base/file_path.h"
#include "gtest/gtest_prod.h" // for FRIEND_TEST
@@ -40,6 +42,7 @@
FRIEND_TEST(CrashCollectorTest, FormatDumpBasename);
FRIEND_TEST(CrashCollectorTest, Initialize);
FRIEND_TEST(CrashCollectorTest, IsCommentLine);
+ FRIEND_TEST(CrashCollectorTest, IsUserSpecificDirectoryEnabled);
FRIEND_TEST(CrashCollectorTest, MetaData);
FRIEND_TEST(CrashCollectorTest, ReadKeyValueFile);
FRIEND_TEST(CrashCollectorTest, Sanitize);
@@ -72,6 +75,8 @@
forced_crash_directory_ = forced_directory;
}
+ virtual GHashTable *GetActiveUserSessions(void);
+ base::FilePath GetUserCrashPath(void);
base::FilePath GetCrashDirectoryInfo(uid_t process_euid,
uid_t default_user_id,
gid_t default_user_group,
diff --git a/crash_reporter/crash_collector_test.cc b/crash_reporter/crash_collector_test.cc
index fff22b5..b9a48a0 100644
--- a/crash_reporter/crash_collector_test.cc
+++ b/crash_reporter/crash_collector_test.cc
@@ -2,8 +2,13 @@
// 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_test.h"
+
#include <unistd.h>
+#include <dbus/dbus-glib-lowlevel.h>
+#include <glib.h>
+
#include "base/file_util.h"
#include "base/string_util.h"
#include "base/stringprintf.h"
@@ -21,6 +26,7 @@
using base::FilePath;
using chromeos::FindLog;
+using ::testing::Return;
void CountCrash() {
ADD_FAILURE();
@@ -48,7 +54,7 @@
bool CheckHasCapacity();
protected:
- CrashCollector collector_;
+ CrashCollectorMock collector_;
FilePath test_dir_;
};
@@ -115,13 +121,25 @@
EXPECT_EQ(kRootUid, directory_owner);
EXPECT_EQ(kRootGid, directory_group);
+ // No need to destroy the hash as GetCrashDirectoryInfo() will do it for us.
+ GHashTable *active_sessions = g_hash_table_new (g_str_hash, g_str_equal);
+ char kUser[] = "chicken@butt.com";
+ char kHash[] = "hashcakes";
+ g_hash_table_insert (active_sessions,
+ static_cast<gpointer>(kUser),
+ static_cast<gpointer>(kHash));
+ EXPECT_CALL(collector_, GetActiveUserSessions())
+ .WillOnce(Return(active_sessions));
+
+ EXPECT_EQ(collector_.IsUserSpecificDirectoryEnabled(), true);
+
path = collector_.GetCrashDirectoryInfo(kChronosUid,
kChronosUid,
kChronosGid,
&directory_mode,
&directory_owner,
&directory_group);
- EXPECT_EQ("/home/chronos/user/crash", path.value());
+ EXPECT_EQ("/home/user/hashcakes", path.value());
EXPECT_EQ(kExpectedUserMode, directory_mode);
EXPECT_EQ(kChronosUid, directory_owner);
EXPECT_EQ(kChronosGid, directory_group);
@@ -344,6 +362,7 @@
}
int main(int argc, char **argv) {
+ ::g_type_init();
SetUpTests(&argc, argv, false);
return RUN_ALL_TESTS();
}
diff --git a/crash_reporter/crash_collector_test.h b/crash_reporter/crash_collector_test.h
new file mode 100644
index 0000000..71b42b7
--- /dev/null
+++ b/crash_reporter/crash_collector_test.h
@@ -0,0 +1,20 @@
+// Copyright (c) 2013 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_TEST_H_
+#define _CRASH_REPORTER_CRASH_COLLECTOR_TEST_H_
+
+#include "crash-reporter/crash_collector.h"
+
+#include <glib.h>
+
+#include <gmock/gmock.h>
+#include <gtest/gtest.h>
+
+class CrashCollectorMock : public CrashCollector {
+ public:
+ MOCK_METHOD0(GetActiveUserSessions, GHashTable *());
+};
+
+#endif // _CRASH_REPORTER_CRASH_COLLECTOR_TEST_H_
diff --git a/crash_reporter/crash_reporter.cc b/crash_reporter/crash_reporter.cc
index 914f711..9bdd564 100644
--- a/crash_reporter/crash_reporter.cc
+++ b/crash_reporter/crash_reporter.cc
@@ -7,6 +7,8 @@
#include <string>
#include <vector>
+#include <glib-object.h>
+
#include <base/file_util.h>
#include <base/command_line.h>
#include <base/logging.h>
@@ -263,6 +265,9 @@
CommandLine::Init(argc, argv);
chromeos::OpenLog(my_path.BaseName().value().c_str(), true);
chromeos::InitLog(chromeos::kLogToSyslog);
+
+ ::g_type_init();
+
KernelCollector kernel_collector;
kernel_collector.Initialize(CountKernelCrash,
IsFeedbackAllowed);
diff --git a/crash_reporter/crash_sender b/crash_reporter/crash_sender
index 97045d4..bb8d04f 100644
--- a/crash_reporter/crash_sender
+++ b/crash_reporter/crash_sender
@@ -492,7 +492,10 @@
send_crashes "/var/spool/crash"
# Send user-specific crashes
- send_crashes "/home/chronos/user/crash"
+ local d
+ for d in /home/chronos/crash /home/chronos/u-*/crash; do
+ send_crashes "${d}"
+ done
}
main
