Do not set properties in bionic_systrace
Currently, if the debug.atrace.tags.enableflags property is not found,
it is set to a safe value such that a pointer to this property can be
stored for later access. This may result in selinux denials because not
all processes write permissions for this property or permission to
connect to the property write socket at all.
Change I6d953c0c281fd72ad3eba8a479fd258023579b5b writes this property to
a safe value upon boot, which greatly decreases the cases in which this
property will not be accessible and removes the need to write it here.
This commit removes this write.
Bug 26115803
Change-Id: Ief72c5f731d3a1231b5080eb531fa0a491a8b1d1
diff --git a/libc/bionic/bionic_systrace.cpp b/libc/bionic/bionic_systrace.cpp
index 103aa8f..b522b10 100644
--- a/libc/bionic/bionic_systrace.cpp
+++ b/libc/bionic/bionic_systrace.cpp
@@ -34,27 +34,25 @@
static Lock g_lock;
static const prop_info* g_pinfo;
-static uint32_t g_serial = -1;
+static uint32_t g_property_serial = -1;
+static uint32_t g_property_area_serial = -1;
static uint64_t g_tags;
static int g_trace_marker_fd = -1;
static bool should_trace() {
bool result = false;
g_lock.lock();
- // If g_pinfo is null, this means that systrace hasn't been run and it's safe to
- // assume that no trace writing will need to take place. However, to avoid running
- // this costly find check each time, we set it to a non-tracing value so that next
- // time, it will just check the serial to see if the value has been changed.
- // this function also deals with the bootup case, during which the call to property
- // set will fail if the property server hasn't yet started.
- if (g_pinfo == NULL) {
+ // debug.atrace.tags.enableflags is set to a safe non-tracing value during property
+ // space initialization, so it should only be null in two cases, if there are
+ // insufficient permissions for this process to access the property, in which
+ // case an audit will be logged, and during boot before the property server has
+ // been started, in which case we store the global property_area serial to prevent
+ // the costly find operation until we see a changed property_area.
+ if (!g_pinfo && g_property_area_serial != __system_property_area_serial()) {
+ g_property_area_serial = __system_property_area_serial();
g_pinfo = __system_property_find(SYSTRACE_PROPERTY_NAME);
- if (g_pinfo == NULL) {
- __system_property_set(SYSTRACE_PROPERTY_NAME, "0");
- g_pinfo = __system_property_find(SYSTRACE_PROPERTY_NAME);
- }
}
- if (g_pinfo != NULL) {
+ if (g_pinfo) {
// Find out which tags have been enabled on the command line and set
// the value of tags accordingly. If the value of the property changes,
// the serial will also change, so the costly system_property_read function
@@ -62,11 +60,11 @@
// first. The values within pinfo may change, but its location is guaranteed
// not to move.
uint32_t cur_serial = __system_property_serial(g_pinfo);
- if (cur_serial != g_serial) {
- g_serial = cur_serial;
+ if (cur_serial != g_property_serial) {
+ g_property_serial = cur_serial;
char value[PROP_VALUE_MAX];
__system_property_read(g_pinfo, 0, value);
- g_tags = strtoull(value, NULL, 0);
+ g_tags = strtoull(value, nullptr, 0);
}
result = ((g_tags & ATRACE_TAG_BIONIC) != 0);
}
