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

Commit 6db7cd78 authored by Steve Fung's avatar Steve Fung
Browse files

crash_reporter: Support crashes from arbitrary users

In order to read the /proc/<pid> files from non-root users without
using CAP_SYS_PTRACE and CAP_DAC_OVERRIDE, use setresuid(..) and
setresgid(..) to switch to the process's user to copy off necessary
files for generating the breakpad minidump.

Bug: 24678424
Change-Id: I4a43583033587441394483ce678c40c4161808b9
parent 7f9d4c97
Loading
Loading
Loading
Loading
+5 −2
Original line number Diff line number Diff line
@@ -14,11 +14,14 @@ on boot
    rmdir /data/misc/crash_reporter/lock/crash_sender

    # Create crash directories.
    mkdir /data/misc/crash_reporter 0700 root root
    # These directories are group-writable by root so that crash_reporter can
    # still access them when it switches users.
    mkdir /data/misc/crash_reporter 0770 root root
    mkdir /data/misc/crash_reporter/crash 0770 root root
    mkdir /data/misc/crash_reporter/lock 0700 root root
    mkdir /data/misc/crash_reporter/log 0700 root root
    mkdir /data/misc/crash_reporter/run 0700 root root
    mkdir /data/misc/crash_reporter/tmp 0700 root root
    mkdir /data/misc/crash_reporter/tmp 0770 root root

service crash_reporter /system/bin/crash_reporter --init
    class late_start
+71 −3
Original line number Diff line number Diff line
@@ -23,9 +23,11 @@
#include <pwd.h>  // For struct passwd.
#include <stdint.h>
#include <sys/cdefs.h>  // For __WORDSIZE
#include <sys/fsuid.h>
#include <sys/types.h>  // For getpwuid_r, getgrnam_r, WEXITSTATUS.
#include <unistd.h>  // For setgroups

#include <iostream>  // For std::oct
#include <string>
#include <vector>

@@ -87,9 +89,9 @@ void UserCollector::Initialize(
  directory_failure_ = directory_failure;
  filter_in_ = filter_in;

  gid_t groups[] = { AID_SYSTEM, AID_DBUS };
  gid_t groups[] = { AID_ROOT, AID_SYSTEM, AID_DBUS };
  if (setgroups(arraysize(groups), groups) != 0) {
    PLOG(FATAL) << "Unable to set groups to system and dbus";
    PLOG(FATAL) << "Unable to set groups to root, system, and dbus";
  }
}

@@ -227,7 +229,19 @@ void UserCollector::EnqueueCollectionErrorLog(pid_t pid,
bool UserCollector::CopyOffProcFiles(pid_t pid,
                                     const FilePath &container_dir) {
  if (!base::CreateDirectory(container_dir)) {
    PLOG(ERROR) << "Could not create " << container_dir.value().c_str();
    PLOG(ERROR) << "Could not create " << container_dir.value();
    return false;
  }
  int dir_mask = base::FILE_PERMISSION_READ_BY_USER
                 | base::FILE_PERMISSION_WRITE_BY_USER
                 | base::FILE_PERMISSION_EXECUTE_BY_USER
                 | base::FILE_PERMISSION_READ_BY_GROUP
                 | base::FILE_PERMISSION_WRITE_BY_GROUP;
  if (!base::SetPosixFilePermissions(container_dir,
                                     base::FILE_PERMISSION_MASK & dir_mask)) {
    PLOG(ERROR) << "Could not set permissions for " << container_dir.value()
                << " to " << std::oct
                << (base::FILE_PERMISSION_MASK & dir_mask);
    return false;
  }
  FilePath process_path = GetProcessPath(pid);
@@ -410,6 +424,43 @@ UserCollector::ErrorType UserCollector::ConvertCoreToMinidump(
  bool proc_files_usable =
      CopyOffProcFiles(pid, container_dir) && ValidateProcFiles(container_dir);

  // Switch back to the original UID/GID.
  gid_t rgid, egid, sgid;
  if (getresgid(&rgid, &egid, &sgid) != 0) {
    PLOG(FATAL) << "Unable to read saved gid";
  }
  if (setresgid(sgid, sgid, -1) != 0) {
    PLOG(FATAL) << "Unable to set real group ID back to saved gid";
  } else {
    if (getresgid(&rgid, &egid, &sgid) != 0) {
      // If the groups cannot be read at this point, the rgid variable will
      // contain the previously read group ID from before changing it.  This
      // will cause the chown call below to set the incorrect group for
      // non-root crashes.  But do not treat this as a fatal error, so that
      // the rest of the collection will continue for potential manual
      // collection by a developer.
      PLOG(ERROR) << "Unable to read real group ID after setting it";
    }
  }

  uid_t ruid, euid, suid;
  if (getresuid(&ruid, &euid, &suid) != 0) {
    PLOG(FATAL) << "Unable to read saved uid";
  }
  if (setresuid(suid, suid, -1) != 0) {
    PLOG(FATAL) << "Unable to set real user ID back to saved uid";
  } else {
    if (getresuid(&ruid, &euid, &suid) != 0) {
      // If the user ID cannot be read at this point, the ruid variable will
      // contain the previously read user ID from before changing it.  This
      // will cause the chown call below to set the incorrect user for
      // non-root crashes.  But do not treat this as a fatal error, so that
      // the rest of the collection will continue for potential manual
      // collection by a developer.
      PLOG(ERROR) << "Unable to read real user ID after setting it";
    }
  }

  if (!CopyStdinToCoreFile(core_path)) {
    return kErrorReadCoreData;
  }
@@ -425,6 +476,13 @@ UserCollector::ErrorType UserCollector::ConvertCoreToMinidump(
    return error;
  }

  // Chown the temp container directory back to the original user/group that
  // crash_reporter is run as, so that additional files can be written to
  // the temp folder.
  if (chown(container_dir.value().c_str(), ruid, rgid) < 0) {
    PLOG(ERROR) << "Could not set owner for " << container_dir.value();
  }

  if (!RunCoreToMinidump(core_path,
                         container_dir,  // procfs directory
                         minidump_path,
@@ -545,6 +603,16 @@ bool UserCollector::HandleCrash(const std::string &crash_attributes,
    return false;
  }

  // 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) {
    PLOG(FATAL) << "Unable to set real group ID to access process files";
  }
  if (setresuid(supplied_ruid, supplied_ruid, -1) != 0) {
    PLOG(FATAL) << "Unable to set real user ID to access process files";
  }

  std::string exec;
  if (force_exec) {
    exec.assign(force_exec);