From 8dfa6b732759f6e20811c3c1d2b830f94d092916 Mon Sep 17 00:00:00 2001 From: Abseil Team Date: Wed, 18 Dec 2024 22:59:00 -0800 Subject: [PATCH] Improve fork-safety by opening files with `O_CLOEXEC`. PiperOrigin-RevId: 707791841 Change-Id: If422c8246c33617e3b103ce5894234d5ee96071f --- absl/debugging/symbolize_elf.inc | 24 +++++++++++++++--------- 1 file changed, 15 insertions(+), 9 deletions(-) diff --git a/absl/debugging/symbolize_elf.inc b/absl/debugging/symbolize_elf.inc index fc046c45940..a98ca81d175 100644 --- a/absl/debugging/symbolize_elf.inc +++ b/absl/debugging/symbolize_elf.inc @@ -378,8 +378,8 @@ class Symbolizer { }; // Protect against client code closing low-valued file descriptors it doesn't -// actually own, as has happened in b/384213477. -int SaferOpen(const char *fname, int flags) { +// actually own. +int OpenReadOnlyWithHighFD(const char *fname) { static int high_fd = [] { struct rlimit rlim{}; const int rc = getrlimit(RLIMIT_NOFILE, &rlim); @@ -393,22 +393,28 @@ int SaferOpen(const char *fname, int flags) { rc, static_cast(rlim.rlim_cur)); return -1; }(); + constexpr int kOpenFlags = O_RDONLY | O_CLOEXEC; if (high_fd >= 1000) { - const int fd = open(fname, flags); + const int fd = open(fname, kOpenFlags); if (fd != -1 && fd < high_fd) { // Try to relocate fd to high range. - const int fd2 = fcntl(fd, F_DUPFD, high_fd); + static_assert(kOpenFlags & O_CLOEXEC, + "F_DUPFD_CLOEXEC assumes O_CLOEXEC"); + const int fd2 = fcntl(fd, F_DUPFD_CLOEXEC, high_fd); if (fd2 != -1) { // Successfully obtained high fd. Use it. close(fd); return fd2; + } else { + ABSL_RAW_LOG(WARNING, "Unable to dup fd=%d above %d, errno=%d", fd, + high_fd, errno); } } // Either open failed and fd==-1, or fd is already above high_fd, or fcntl // failed and fd is valid (but low). return fd; } - return open(fname, flags); + return open(fname, kOpenFlags); } static std::atomic g_cached_symbolizer; @@ -1099,7 +1105,7 @@ static ABSL_ATTRIBUTE_NOINLINE bool ReadAddrMap( snprintf(maps_path, sizeof(maps_path), "/proc/self/task/%d/maps", getpid()); int maps_fd; - NO_INTR(maps_fd = SaferOpen(maps_path, O_RDONLY)); + NO_INTR(maps_fd = OpenReadOnlyWithHighFD(maps_path)); FileDescriptor wrapped_maps_fd(maps_fd); if (wrapped_maps_fd.get() < 0) { ABSL_RAW_LOG(WARNING, "%s: errno=%d", maps_path, errno); @@ -1373,7 +1379,7 @@ static void MaybeOpenFdFromSelfExe(ObjFile *obj) { if (memcmp(obj->start_addr, ELFMAG, SELFMAG) != 0) { return; } - int fd = SaferOpen("/proc/self/exe", O_RDONLY); + int fd = OpenReadOnlyWithHighFD("/proc/self/exe"); if (fd == -1) { return; } @@ -1397,7 +1403,7 @@ static void MaybeOpenFdFromSelfExe(ObjFile *obj) { static bool MaybeInitializeObjFile(ObjFile *obj) { if (obj->fd < 0) { - obj->fd = SaferOpen(obj->filename, O_RDONLY); + obj->fd = OpenReadOnlyWithHighFD(obj->filename); if (obj->fd < 0) { // Getting /proc/self/exe here means that we were hinted. @@ -1405,7 +1411,7 @@ static bool MaybeInitializeObjFile(ObjFile *obj) { // /proc/self/exe may be inaccessible (due to setuid, etc.), so try // accessing the binary via argv0. if (argv0_value != nullptr) { - obj->fd = SaferOpen(argv0_value, O_RDONLY); + obj->fd = OpenReadOnlyWithHighFD(argv0_value); } } else { MaybeOpenFdFromSelfExe(obj);