Merge "Update metrics to use weaved's client library"
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) {
diff --git a/debuggerd/debuggerd.cpp b/debuggerd/debuggerd.cpp
index 599995c..713638d 100644
--- a/debuggerd/debuggerd.cpp
+++ b/debuggerd/debuggerd.cpp
@@ -130,31 +130,44 @@
"dump_backtrace"
};
-static bool selinux_action_allowed(int s, pid_t tid, debugger_action_t action)
+static int audit_callback(void* data, security_class_t /* cls */, char* buf, size_t len)
+{
+ struct debugger_request_t* req = reinterpret_cast<debugger_request_t*>(data);
+
+ if (!req) {
+ ALOGE("No debuggerd request audit data");
+ return 0;
+ }
+
+ snprintf(buf, len, "pid=%d uid=%d gid=%d", req->pid, req->uid, req->gid);
+ return 0;
+}
+
+static bool selinux_action_allowed(int s, debugger_request_t* request)
{
char *scon = NULL, *tcon = NULL;
const char *tclass = "debuggerd";
const char *perm;
bool allowed = false;
- if (action <= 0 || action >= (sizeof(debuggerd_perms)/sizeof(debuggerd_perms[0]))) {
- ALOGE("SELinux: No permission defined for debugger action %d", action);
+ if (request->action <= 0 || request->action >= (sizeof(debuggerd_perms)/sizeof(debuggerd_perms[0]))) {
+ ALOGE("SELinux: No permission defined for debugger action %d", request->action);
return false;
}
- perm = debuggerd_perms[action];
+ perm = debuggerd_perms[request->action];
if (getpeercon(s, &scon) < 0) {
ALOGE("Cannot get peer context from socket\n");
goto out;
}
- if (getpidcon(tid, &tcon) < 0) {
- ALOGE("Cannot get context for tid %d\n", tid);
+ if (getpidcon(request->tid, &tcon) < 0) {
+ ALOGE("Cannot get context for tid %d\n", request->tid);
goto out;
}
- allowed = (selinux_check_access(scon, tcon, tclass, perm, NULL) == 0);
+ allowed = (selinux_check_access(scon, tcon, tclass, perm, reinterpret_cast<void*>(request)) == 0);
out:
freecon(scon);
@@ -225,7 +238,7 @@
return -1;
}
- if (!selinux_action_allowed(fd, out_request->tid, out_request->action))
+ if (!selinux_action_allowed(fd, out_request))
return -1;
} else {
// No one else is allowed to dump arbitrary processes.
@@ -566,6 +579,8 @@
int main(int argc, char** argv) {
union selinux_callback cb;
if (argc == 1) {
+ cb.func_audit = audit_callback;
+ selinux_set_callback(SELINUX_CB_AUDIT, cb);
cb.func_log = selinux_log_callback;
selinux_set_callback(SELINUX_CB_LOG, cb);
return do_server();
diff --git a/init/init.cpp b/init/init.cpp
index ee1351d..a898b03 100644
--- a/init/init.cpp
+++ b/init/init.cpp
@@ -467,7 +467,16 @@
}
static int audit_callback(void *data, security_class_t /*cls*/, char *buf, size_t len) {
- snprintf(buf, len, "property=%s", !data ? "NULL" : (char *)data);
+
+ property_audit_data *d = reinterpret_cast<property_audit_data*>(data);
+
+ if (!d || !d->name || !d->cr) {
+ ERROR("audit_callback invoked with null data arguments!");
+ return 0;
+ }
+
+ snprintf(buf, len, "property=%s pid=%d uid=%d gid=%d", d->name,
+ d->cr->pid, d->cr->uid, d->cr->gid);
return 0;
}
diff --git a/init/property_service.cpp b/init/property_service.cpp
index 7194820..ebf768e 100644
--- a/init/property_service.cpp
+++ b/init/property_service.cpp
@@ -90,10 +90,11 @@
}
}
-static int check_mac_perms(const char *name, char *sctx)
+static int check_mac_perms(const char *name, char *sctx, struct ucred *cr)
{
char *tctx = NULL;
int result = 0;
+ property_audit_data audit_data;
if (!sctx)
goto err;
@@ -104,7 +105,10 @@
if (selabel_lookup(sehandle_prop, &tctx, name, 1) != 0)
goto err;
- if (selinux_check_access(sctx, tctx, "property_service", "set", (void*) name) == 0)
+ audit_data.name = name;
+ audit_data.cr = cr;
+
+ if (selinux_check_access(sctx, tctx, "property_service", "set", reinterpret_cast<void*>(&audit_data)) == 0)
result = 1;
freecon(tctx);
@@ -112,7 +116,7 @@
return result;
}
-static int check_control_mac_perms(const char *name, char *sctx)
+static int check_control_mac_perms(const char *name, char *sctx, struct ucred *cr)
{
/*
* Create a name prefix out of ctl.<service name>
@@ -126,19 +130,19 @@
if (ret < 0 || (size_t) ret >= sizeof(ctl_name))
return 0;
- return check_mac_perms(ctl_name, sctx);
+ return check_mac_perms(ctl_name, sctx, cr);
}
/*
* Checks permissions for setting system properties.
* Returns 1 if uid allowed, 0 otherwise.
*/
-static int check_perms(const char *name, char *sctx)
+static int check_perms(const char *name, char *sctx, struct ucred *cr)
{
if(!strncmp(name, "ro.", 3))
name +=3;
- return check_mac_perms(name, sctx);
+ return check_mac_perms(name, sctx, cr);
}
std::string property_get(const char* name) {
@@ -321,14 +325,14 @@
// Keep the old close-socket-early behavior when handling
// ctl.* properties.
close(s);
- if (check_control_mac_perms(msg.value, source_ctx)) {
+ if (check_control_mac_perms(msg.value, source_ctx, &cr)) {
handle_control_message((char*) msg.name + 4, (char*) msg.value);
} else {
ERROR("sys_prop: Unable to %s service ctl [%s] uid:%d gid:%d pid:%d\n",
msg.name + 4, msg.value, cr.uid, cr.gid, cr.pid);
}
} else {
- if (check_perms(msg.name, source_ctx)) {
+ if (check_perms(msg.name, source_ctx, &cr)) {
property_set((char*) msg.name, (char*) msg.value);
} else {
ERROR("sys_prop: permission denied uid:%d name:%s\n",
diff --git a/init/property_service.h b/init/property_service.h
index 51d7404..1a48fb1 100644
--- a/init/property_service.h
+++ b/init/property_service.h
@@ -18,9 +18,15 @@
#define _INIT_PROPERTY_H
#include <stddef.h>
+#include <sys/socket.h>
#include <sys/system_properties.h>
#include <string>
+struct property_audit_data {
+ ucred *cr;
+ const char* name;
+};
+
extern void property_init(void);
extern void property_load_boot_defaults(void);
extern void load_persist_props(void);