Skip to content

Commit 8dfa6b7

Browse files
Abseil Teamcopybara-github
authored andcommitted
Improve fork-safety by opening files with O_CLOEXEC.
PiperOrigin-RevId: 707791841 Change-Id: If422c8246c33617e3b103ce5894234d5ee96071f
1 parent f0e5905 commit 8dfa6b7

File tree

1 file changed

+15
-9
lines changed

1 file changed

+15
-9
lines changed

absl/debugging/symbolize_elf.inc

Lines changed: 15 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -378,8 +378,8 @@ class Symbolizer {
378378
};
379379

380380
// Protect against client code closing low-valued file descriptors it doesn't
381-
// actually own, as has happened in b/384213477.
382-
int SaferOpen(const char *fname, int flags) {
381+
// actually own.
382+
int OpenReadOnlyWithHighFD(const char *fname) {
383383
static int high_fd = [] {
384384
struct rlimit rlim{};
385385
const int rc = getrlimit(RLIMIT_NOFILE, &rlim);
@@ -393,22 +393,28 @@ int SaferOpen(const char *fname, int flags) {
393393
rc, static_cast<long>(rlim.rlim_cur));
394394
return -1;
395395
}();
396+
constexpr int kOpenFlags = O_RDONLY | O_CLOEXEC;
396397
if (high_fd >= 1000) {
397-
const int fd = open(fname, flags);
398+
const int fd = open(fname, kOpenFlags);
398399
if (fd != -1 && fd < high_fd) {
399400
// Try to relocate fd to high range.
400-
const int fd2 = fcntl(fd, F_DUPFD, high_fd);
401+
static_assert(kOpenFlags & O_CLOEXEC,
402+
"F_DUPFD_CLOEXEC assumes O_CLOEXEC");
403+
const int fd2 = fcntl(fd, F_DUPFD_CLOEXEC, high_fd);
401404
if (fd2 != -1) {
402405
// Successfully obtained high fd. Use it.
403406
close(fd);
404407
return fd2;
408+
} else {
409+
ABSL_RAW_LOG(WARNING, "Unable to dup fd=%d above %d, errno=%d", fd,
410+
high_fd, errno);
405411
}
406412
}
407413
// Either open failed and fd==-1, or fd is already above high_fd, or fcntl
408414
// failed and fd is valid (but low).
409415
return fd;
410416
}
411-
return open(fname, flags);
417+
return open(fname, kOpenFlags);
412418
}
413419

414420
static std::atomic<Symbolizer *> g_cached_symbolizer;
@@ -1099,7 +1105,7 @@ static ABSL_ATTRIBUTE_NOINLINE bool ReadAddrMap(
10991105
snprintf(maps_path, sizeof(maps_path), "/proc/self/task/%d/maps", getpid());
11001106

11011107
int maps_fd;
1102-
NO_INTR(maps_fd = SaferOpen(maps_path, O_RDONLY));
1108+
NO_INTR(maps_fd = OpenReadOnlyWithHighFD(maps_path));
11031109
FileDescriptor wrapped_maps_fd(maps_fd);
11041110
if (wrapped_maps_fd.get() < 0) {
11051111
ABSL_RAW_LOG(WARNING, "%s: errno=%d", maps_path, errno);
@@ -1373,7 +1379,7 @@ static void MaybeOpenFdFromSelfExe(ObjFile *obj) {
13731379
if (memcmp(obj->start_addr, ELFMAG, SELFMAG) != 0) {
13741380
return;
13751381
}
1376-
int fd = SaferOpen("/proc/self/exe", O_RDONLY);
1382+
int fd = OpenReadOnlyWithHighFD("/proc/self/exe");
13771383
if (fd == -1) {
13781384
return;
13791385
}
@@ -1397,15 +1403,15 @@ static void MaybeOpenFdFromSelfExe(ObjFile *obj) {
13971403

13981404
static bool MaybeInitializeObjFile(ObjFile *obj) {
13991405
if (obj->fd < 0) {
1400-
obj->fd = SaferOpen(obj->filename, O_RDONLY);
1406+
obj->fd = OpenReadOnlyWithHighFD(obj->filename);
14011407

14021408
if (obj->fd < 0) {
14031409
// Getting /proc/self/exe here means that we were hinted.
14041410
if (strcmp(obj->filename, "/proc/self/exe") == 0) {
14051411
// /proc/self/exe may be inaccessible (due to setuid, etc.), so try
14061412
// accessing the binary via argv0.
14071413
if (argv0_value != nullptr) {
1408-
obj->fd = SaferOpen(argv0_value, O_RDONLY);
1414+
obj->fd = OpenReadOnlyWithHighFD(argv0_value);
14091415
}
14101416
} else {
14111417
MaybeOpenFdFromSelfExe(obj);

0 commit comments

Comments
 (0)