Donate to e Foundation | Murena handsets with /e/OS | Own a part of Murena! Learn more

Commit 773fd3c4 authored by Steve Fung's avatar Steve Fung
Browse files

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
parent a3ae129f

crash_reporter/crash_reporter.rc

+1 −1
Original line number Diff line number Diff line 
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"

crash_reporter/user_collector.cc

+11 −22
Original line number Diff line number Diff line
@@ -51,8 +51,9 @@ static const char kStatePrefix[] = "State:\t"; 


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 @@ std::string UserCollector::GetErrorTypeSignature(ErrorType error_type) const { 
  }

}



// 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 @@ UserCollector::ErrorType UserCollector::ConvertAndEnqueueCrash( 


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 @@ bool UserCollector::HandleCrash(const std::string &crash_attributes, 
  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 @@ bool UserCollector::HandleCrash(const std::string &crash_attributes, 
  // 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) {

crash_reporter/user_collector.h

+1 −2
Original line number Diff line number Diff line
@@ -105,7 +105,6 @@ class UserCollector : public CrashCollector { 
  // 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 @@ class UserCollector : public CrashCollector { 
  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,

crash_reporter/user_collector_test.cc

+11 −8
Original line number Diff line number Diff line
@@ -98,35 +98,38 @@ TEST_F(UserCollectorTest, ParseCrashAttributes) { 
  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) {