From 58aac1bd87ce40f0f67af1a504c278509412555b Mon Sep 17 00:00:00 2001 From: Mark Mentovai Date: Tue, 28 Feb 2017 16:15:25 -0500 Subject: [PATCH] Set FD_CLOEXEC on file descriptors obtained from open() and fopen() Includes an update of mini_chromium to 3a2d52d74c9a: 3a2d52d74c9a Use O_CLOEXEC (and O_NOCTTY) when calling open() BUG=chromium:688362 Change-Id: I2bdf86efe4e6559ecb77492ac5bdc728aa035889 Reviewed-on: https://chromium-review.googlesource.com/447999 Reviewed-by: Scott Graham --- DEPS | 2 +- client/crash_report_database_mac.mm | 12 +++++++----- tools/mac/catch_exception_tool.cc | 5 +++++ util/file/file_io_posix.cc | 8 +++++--- util/mac/xattr_test.cc | 6 ++++-- util/posix/close_multiple.cc | 2 +- util/posix/close_stdio.cc | 3 ++- util/posix/process_info_test.cc | 2 +- 8 files changed, 26 insertions(+), 14 deletions(-) diff --git a/DEPS b/DEPS index f5e26023..a2def66c 100644 --- a/DEPS +++ b/DEPS @@ -38,7 +38,7 @@ deps = { 'crashpad/third_party/mini_chromium/mini_chromium': Var('chromium_git') + '/chromium/mini_chromium@' + - 'f65519e442d23498937251e680a3b113927613b0', + '3a2d52d74c9af5277bf6456cc00ae728f89c4898', 'crashpad/third_party/zlib/zlib': Var('chromium_git') + '/chromium/src/third_party/zlib@' + '13dc246a58e4b72104d35f9b1809af95221ebda7', diff --git a/client/crash_report_database_mac.mm b/client/crash_report_database_mac.mm index f9c783eb..0d074cba 100644 --- a/client/crash_report_database_mac.mm +++ b/client/crash_report_database_mac.mm @@ -299,9 +299,10 @@ CrashReportDatabaseMac::PrepareNewCrashReport(NewReport** out_report) { base_dir_.Append(kWriteDirectory) .Append(report->uuid.ToString() + "." + kCrashReportFileExtension); - report->handle = HANDLE_EINTR(open(report->path.value().c_str(), - O_CREAT | O_WRONLY | O_EXCL | O_EXLOCK, - 0600)); + report->handle = HANDLE_EINTR( + open(report->path.value().c_str(), + O_WRONLY | O_EXLOCK | O_CREAT | O_EXCL | O_NOCTTY | O_CLOEXEC, + 0600)); if (report->handle < 0) { PLOG(ERROR) << "open " << report->path.value(); return kFileSystemError; @@ -612,8 +613,9 @@ CrashReportDatabase::OperationStatus CrashReportDatabaseMac::RequestUpload( // static base::ScopedFD CrashReportDatabaseMac::ObtainReportLock( const base::FilePath& path) { - int fd = HANDLE_EINTR(open(path.value().c_str(), - O_RDONLY | O_EXLOCK | O_NONBLOCK)); + int fd = HANDLE_EINTR( + open(path.value().c_str(), + O_RDONLY | O_NONBLOCK | O_EXLOCK | O_NOCTTY | O_CLOEXEC)); PLOG_IF(ERROR, fd < 0) << "open lock " << path.value(); return base::ScopedFD(fd); } diff --git a/tools/mac/catch_exception_tool.cc b/tools/mac/catch_exception_tool.cc index afd738a4..3a535cb2 100644 --- a/tools/mac/catch_exception_tool.cc +++ b/tools/mac/catch_exception_tool.cc @@ -12,6 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. +#include #include #include #include @@ -279,6 +280,10 @@ int CatchExceptionToolMain(int argc, char* argv[]) { return EXIT_FAILURE; } options.file = file_owner.get(); + if (fcntl(fileno(options.file), F_SETFD, FD_CLOEXEC) == -1) { + PLOG(ERROR) << "fcntl " << options.file_path; + return EXIT_FAILURE; + } } int exceptions_handled = 0; diff --git a/util/file/file_io_posix.cc b/util/file/file_io_posix.cc index 30eae539..1534db15 100644 --- a/util/file/file_io_posix.cc +++ b/util/file/file_io_posix.cc @@ -80,10 +80,11 @@ FileHandle OpenFileForOutput(int rdwr_or_wronly, const base::FilePath& path, FileWriteMode mode, FilePermissions permissions) { + int flags = O_NOCTTY | O_CLOEXEC; + DCHECK(rdwr_or_wronly & (O_RDWR | O_WRONLY)); DCHECK_EQ(rdwr_or_wronly & ~(O_RDWR | O_WRONLY), 0); - - int flags = rdwr_or_wronly; + flags |= rdwr_or_wronly; switch (mode) { case FileWriteMode::kReuseOrFail: @@ -118,7 +119,8 @@ FileOperationResult WriteFile(FileHandle file, } FileHandle OpenFileForRead(const base::FilePath& path) { - return HANDLE_EINTR(open(path.value().c_str(), O_RDONLY)); + return HANDLE_EINTR( + open(path.value().c_str(), O_RDONLY | O_NOCTTY | O_CLOEXEC)); } FileHandle OpenFileForWrite(const base::FilePath& path, diff --git a/util/mac/xattr_test.cc b/util/mac/xattr_test.cc index daaba307..c27ab4c1 100644 --- a/util/mac/xattr_test.cc +++ b/util/mac/xattr_test.cc @@ -37,8 +37,10 @@ class Xattr : public testing::Test { void SetUp() override { path_ = temp_dir_.path().Append("xattr_file"); - base::ScopedFD tmp(HANDLE_EINTR( - open(path_.value().c_str(), O_CREAT | O_TRUNC, 0644))); + base::ScopedFD tmp( + HANDLE_EINTR(open(path_.value().c_str(), + O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY | O_CLOEXEC, + 0644))); EXPECT_GE(tmp.get(), 0) << ErrnoMessage("open"); } diff --git a/util/posix/close_multiple.cc b/util/posix/close_multiple.cc index f1d7773f..172a8f02 100644 --- a/util/posix/close_multiple.cc +++ b/util/posix/close_multiple.cc @@ -211,7 +211,7 @@ void CloseMultipleNowOrOnExec(int fd, int preserve_fd) { // do_prlimit() and kernel/sysctl.c fs_table. Inability to open this file is // not considered an error, because /proc may not be available or usable. { - base::ScopedFILE nr_open_file(fopen("/proc/sys/fs/nr_open", "r")); + base::ScopedFILE nr_open_file(fopen("/proc/sys/fs/nr_open", "re")); if (nr_open_file.get() != nullptr) { int nr_open; if (fscanf(nr_open_file.get(), "%d\n", &nr_open) == 1 && diff --git a/util/posix/close_stdio.cc b/util/posix/close_stdio.cc index d5892947..cc9cdac8 100644 --- a/util/posix/close_stdio.cc +++ b/util/posix/close_stdio.cc @@ -27,7 +27,8 @@ namespace crashpad { namespace { void CloseStdioStream(int desired_fd, int oflag) { - base::ScopedFD fd(HANDLE_EINTR(open(_PATH_DEVNULL, oflag))); + base::ScopedFD fd( + HANDLE_EINTR(open(_PATH_DEVNULL, oflag | O_NOCTTY | O_CLOEXEC))); if (fd == desired_fd) { // Weird, but play along. ignore_result(fd.release()); diff --git a/util/posix/process_info_test.cc b/util/posix/process_info_test.cc index 7e28e5f4..c07b1ef7 100644 --- a/util/posix/process_info_test.cc +++ b/util/posix/process_info_test.cc @@ -103,7 +103,7 @@ void TestSelfProcess(const ProcessInfo& process_info) { #elif defined(OS_LINUX) || defined(OS_ANDROID) std::vector expect_arg_vector; { - base::ScopedFILE cmdline(fopen("/proc/self/cmdline", "r")); + base::ScopedFILE cmdline(fopen("/proc/self/cmdline", "re")); ASSERT_NE(nullptr, cmdline.get()) << ErrnoMessage("fopen"); int expect_arg_char;