From 88846a2ccfdbe36bcb04fa3f4a5b735403e8a1b6 Mon Sep 17 00:00:00 2001 From: Josh Gao Date: Mon, 1 Feb 2021 16:48:25 -0800 Subject: [PATCH] Let system_server truncate tombstones. There's no way to atomically unlink a specific file for which we have an fd from a path, which means that we can't safely delete a tombstone without coordination with tombstoned, which is risky. For example, if we use flock on the directory, and system_server crashes while holding the lock, we risk deadlock. We do the next best thing, and keep a file descriptor around for every tombstone, and truncate it, which requires system_server to be able to write to tombstones (which are owned by the system group). Test: treehugger Change-Id: I6ba7f1fe87ee1a4b57bdb3741e8ec9fbc80788c9 --- debuggerd/tombstoned/tombstoned.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/debuggerd/tombstoned/tombstoned.cpp b/debuggerd/tombstoned/tombstoned.cpp index f057260e1..436f6c9cc 100644 --- a/debuggerd/tombstoned/tombstoned.cpp +++ b/debuggerd/tombstoned/tombstoned.cpp @@ -143,13 +143,13 @@ class CrashQueue { CrashArtifact result; std::optional path; - result.fd.reset(openat(dir_fd_, ".", O_WRONLY | O_APPEND | O_TMPFILE | O_CLOEXEC, 0640)); + result.fd.reset(openat(dir_fd_, ".", O_WRONLY | O_APPEND | O_TMPFILE | O_CLOEXEC, 0660)); if (result.fd == -1) { // We might not have O_TMPFILE. Try creating with an arbitrary filename instead. static size_t counter = 0; std::string tmp_filename = StringPrintf(".temporary%zu", counter++); result.fd.reset(openat(dir_fd_, tmp_filename.c_str(), - O_WRONLY | O_APPEND | O_CREAT | O_TRUNC | O_CLOEXEC, 0640)); + O_WRONLY | O_APPEND | O_CREAT | O_TRUNC | O_CLOEXEC, 0660)); if (result.fd == -1) { PLOG(FATAL) << "failed to create temporary tombstone in " << dir_path_; } @@ -509,7 +509,7 @@ static void crash_completed_cb(evutil_socket_t sockfd, short ev, void* arg) { } int main(int, char* []) { - umask(0137); + umask(0117); // Don't try to connect to ourselves if we crash. struct sigaction action = {};