profiling: override dumpability while opening /proc/self/mem,maps
For the perf profiling signal handler to succeed in opening
/proc/self/mem, the process needs to be marked as dumpable in posix
terms. This patch addresses a scenario since Android S where the process
is considered profileable, but is not dumpable on "user" builds. The
solution is to mark the process as dumpable while opening the procfs
descriptors, restoring the original value afterwards. This is the same
approach as the heapprofd heap profiler, which performs the override
within the loaded client library [1].
The particular scenario being addressed is:
* user build
* app does not explicitly opt into being profiled by shell
* app does not explicitly opt out of all profiling
In this case, the app is considered profileable by the platform (but NOT
shell). Therefore ActivityThread marks the process as profileable [2],
but the zygote keeps the process as undumpable as it considers the
profileability from the shell domain [3]. We could change the logic in
the zygote to leave such processes in the dumpable state, but the
override within the signal handler is considered to be more contained as
the dumpability is only needed temporarily.
This override would also apply for any non-dumpable native services that
are signalled for profiling, which is also desireable for profiling
coverage.
This change does not elide any of the existing profileability
checks by the signal handler's preamble and the profiler itself.
[1]
https://cs.android.com/android/platform/superproject/+/master:external/perfetto/src/profiling/memory/client.cc;l=184;drc=78cd82ba31233ce810618e07d349fd34efdb861d
[2]
https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/java/android/app/ActivityThread.java;l=6610;drc=de9cf3392d7872c2bee69b65a614e77bb166b26e
[3]
https://cs.android.com/android/platform/superproject/+/master:frameworks/base/core/jni/com_android_internal_os_Zygote.cpp;l=1680;drc=master
Tested: clock app on barbet-user succeeds in opening the procfs
descriptors within the signal handler.
Tested: systemwide profiling on sargo-userdebug works as before.
Bug: 196810669
BYPASS_INCLUSIVE_LANGUAGE_REASON=referencing the name of a cmdline utility
Change-Id: Id621d4312418ff0736c97065e9ee577ff67f40da
diff --git a/libc/bionic/android_profiling_dynamic.cpp b/libc/bionic/android_profiling_dynamic.cpp
index 4fafd67..3460a6d 100644
--- a/libc/bionic/android_profiling_dynamic.cpp
+++ b/libc/bionic/android_profiling_dynamic.cpp
@@ -33,6 +33,7 @@
#include <fcntl.h>
#include <signal.h>
#include <string.h>
+#include <sys/prctl.h>
#include <sys/socket.h>
#include <sys/stat.h>
#include <sys/types.h>
@@ -137,13 +138,25 @@
return;
}
+ // If the process is undumpable, /proc/self/mem will be owned by root:root, and therefore
+ // inaccessible to the process itself (see man 5 proc). We temporarily mark the process as
+ // dumpable to allow for the open. Note: prctl is not async signal safe per posix, but bionic's
+ // implementation is. Error checking on prctls is omitted due to them being trivial.
+ int orig_dumpable = prctl(PR_GET_DUMPABLE, 0, 0, 0, 0);
+ if (!orig_dumpable) {
+ prctl(PR_SET_DUMPABLE, 1, 0, 0, 0);
+ }
ScopedFd maps_fd{ open("/proc/self/maps", O_RDONLY | O_CLOEXEC) };
+ ScopedFd mem_fd{ open("/proc/self/mem", O_RDONLY | O_CLOEXEC) };
+ if (!orig_dumpable) {
+ prctl(PR_SET_DUMPABLE, orig_dumpable, 0, 0, 0);
+ }
+
if (maps_fd.get() == -1) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "failed to open /proc/self/maps: %s",
strerror(errno));
return;
}
- ScopedFd mem_fd{ open("/proc/self/mem", O_RDONLY | O_CLOEXEC) };
if (mem_fd.get() == -1) {
async_safe_format_log(ANDROID_LOG_ERROR, "libc", "failed to open /proc/self/mem: %s",
strerror(errno));