crash_reporter: Use the actual GID of the crashing process
Rather than assuming the UID and GID of crashing processes is
the same, report and use the actual GID that the process was
running as.
Bug: 24678424
Change-Id: I3cfc415be2feb2863a4f4b850bfd4a3267217a44
diff --git a/crash_reporter/crash_reporter.rc b/crash_reporter/crash_reporter.rc
index 5fb0d7a..7bfe0c2 100644
--- a/crash_reporter/crash_reporter.rc
+++ b/crash_reporter/crash_reporter.rc
@@ -1,6 +1,6 @@
on property:crash_reporter.coredump.enabled=1
write /proc/sys/kernel/core_pattern \
- "|/system/bin/crash_reporter --user=%P:%s:%u:%e"
+ "|/system/bin/crash_reporter --user=%P:%s:%u:%g:%e"
on property:crash_reporter.coredump.enabled=0
write /proc/sys/kernel/core_pattern "core"
diff --git a/crash_reporter/user_collector.cc b/crash_reporter/user_collector.cc
index e27c905..c4f02af 100644
--- a/crash_reporter/user_collector.cc
+++ b/crash_reporter/user_collector.cc
@@ -51,8 +51,9 @@
static const char kCoreTempFolder[] = "/data/misc/crash_reporter/tmp";
-// Define an otherwise invalid value that represents an unknown UID.
+// Define an otherwise invalid value that represents an unknown UID and GID.
static const uid_t kUnknownUid = -1;
+static const gid_t kUnknownGid = -1;
const char *UserCollector::kUserId = "Uid:\t";
const char *UserCollector::kGroupId = "Gid:\t";
@@ -117,22 +118,6 @@
}
}
-// Return the string that should be used for the kernel's core_pattern file.
-// Note that if you change the format of the enabled pattern, you'll probably
-// also need to change the ParseCrashAttributes() function below, the
-// user_collector_test.cc unittest, and the logging_UserCrash.py autotest.
-std::string UserCollector::GetPattern(bool enabled) const {
- if (enabled) {
- // Combine the four crash attributes into one parameter to try to reduce
- // the size of the invocation line for crash_reporter, since the kernel
- // has a fixed-sized (128B) buffer for it (before parameter expansion).
- // Note that the kernel does not support quoted arguments in core_pattern.
- return StringPrintf("|%s --user=%%P:%%s:%%u:%%e", our_path_.c_str());
- } else {
- return "core";
- }
-}
-
bool UserCollector::SetUpInternal(bool enabled) {
CHECK(initialized_);
LOG(INFO) << (enabled ? "Enabling" : "Disabling") << " user crash handling";
@@ -554,15 +539,18 @@
bool UserCollector::ParseCrashAttributes(const std::string &crash_attributes,
pid_t *pid, int *signal, uid_t *uid,
+ gid_t *gid,
std::string *kernel_supplied_name) {
- pcrecpp::RE re("(\\d+):(\\d+):(\\d+):(.*)");
- if (re.FullMatch(crash_attributes, pid, signal, uid, kernel_supplied_name))
+ pcrecpp::RE re("(\\d+):(\\d+):(\\d+):(\\d+):(.*)");
+ if (re.FullMatch(crash_attributes, pid, signal, uid, gid,
+ kernel_supplied_name))
return true;
LOG(INFO) << "Falling back to parsing crash attributes '"
- << crash_attributes << "' without UID";
+ << crash_attributes << "' without UID and GID";
pcrecpp::RE re_without_uid("(\\d+):(\\d+):(.*)");
*uid = kUnknownUid;
+ *gid = kUnknownGid;
return re_without_uid.FullMatch(crash_attributes, pid, signal,
kernel_supplied_name);
}
@@ -595,10 +583,11 @@
pid_t pid = 0;
int signal = 0;
uid_t supplied_ruid = kUnknownUid;
+ gid_t supplied_rgid = kUnknownGid;
std::string kernel_supplied_name;
if (!ParseCrashAttributes(crash_attributes, &pid, &signal, &supplied_ruid,
- &kernel_supplied_name)) {
+ &supplied_rgid, &kernel_supplied_name)) {
LOG(ERROR) << "Invalid parameter: --user=" << crash_attributes;
return false;
}
@@ -606,7 +595,7 @@
// Switch to the group and user that ran the crashing binary in order to
// access their /proc files. Do not set suid/sgid, so that we can switch
// back after copying the necessary files.
- if (setresgid(supplied_ruid, supplied_ruid, -1) != 0) {
+ if (setresgid(supplied_rgid, supplied_rgid, -1) != 0) {
PLOG(FATAL) << "Unable to set real group ID to access process files";
}
if (setresuid(supplied_ruid, supplied_ruid, -1) != 0) {
diff --git a/crash_reporter/user_collector.h b/crash_reporter/user_collector.h
index 8c38aa2..7261ed4 100644
--- a/crash_reporter/user_collector.h
+++ b/crash_reporter/user_collector.h
@@ -105,7 +105,6 @@
// crash_reporter-user-collection signature.
std::string GetErrorTypeSignature(ErrorType error_type) const;
- std::string GetPattern(bool enabled) const;
bool SetUpInternal(bool enabled);
// Returns, via |line|, the first line in |lines| that starts with |prefix|.
@@ -166,7 +165,7 @@
ErrorType ConvertAndEnqueueCrash(pid_t pid, const std::string &exec_name,
uid_t supplied_ruid, bool *out_of_capacity);
bool ParseCrashAttributes(const std::string &crash_attributes,
- pid_t *pid, int *signal, uid_t *uid,
+ pid_t *pid, int *signal, uid_t *uid, gid_t *gid,
std::string *kernel_supplied_name);
bool ShouldDump(bool has_owner_consent,
diff --git a/crash_reporter/user_collector_test.cc b/crash_reporter/user_collector_test.cc
index 4419e7c..1dfac2a 100644
--- a/crash_reporter/user_collector_test.cc
+++ b/crash_reporter/user_collector_test.cc
@@ -98,35 +98,38 @@
pid_t pid;
int signal;
uid_t uid;
+ gid_t gid;
std::string exec_name;
- EXPECT_TRUE(collector_.ParseCrashAttributes("123456:11:1000:foobar",
- &pid, &signal, &uid, &exec_name));
+ EXPECT_TRUE(collector_.ParseCrashAttributes("123456:11:1000:2000:foobar",
+ &pid, &signal, &uid, &gid, &exec_name));
EXPECT_EQ(123456, pid);
EXPECT_EQ(11, signal);
EXPECT_EQ(1000, uid);
+ EXPECT_EQ(2000, gid);
EXPECT_EQ("foobar", exec_name);
EXPECT_TRUE(collector_.ParseCrashAttributes("4321:6:barfoo",
- &pid, &signal, &uid, &exec_name));
+ &pid, &signal, &uid, &gid, &exec_name));
EXPECT_EQ(4321, pid);
EXPECT_EQ(6, signal);
EXPECT_EQ(-1, uid);
+ EXPECT_EQ(-1, gid);
EXPECT_EQ("barfoo", exec_name);
EXPECT_FALSE(collector_.ParseCrashAttributes("123456:11",
- &pid, &signal, &uid, &exec_name));
+ &pid, &signal, &uid, &gid, &exec_name));
EXPECT_TRUE(collector_.ParseCrashAttributes("123456:11:exec:extra",
- &pid, &signal, &uid, &exec_name));
+ &pid, &signal, &uid, &gid, &exec_name));
EXPECT_EQ("exec:extra", exec_name);
EXPECT_FALSE(collector_.ParseCrashAttributes("12345p:11:foobar",
- &pid, &signal, &uid, &exec_name));
+ &pid, &signal, &uid, &gid, &exec_name));
EXPECT_FALSE(collector_.ParseCrashAttributes("123456:1 :foobar",
- &pid, &signal, &uid, &exec_name));
+ &pid, &signal, &uid, &gid, &exec_name));
EXPECT_FALSE(collector_.ParseCrashAttributes("123456::foobar",
- &pid, &signal, &uid, &exec_name));
+ &pid, &signal, &uid, &gid, &exec_name));
}
TEST_F(UserCollectorTest, ShouldDumpDeveloperImageOverridesConsent) {