From 7500e2ef452a8902962bf89623d82e47ee023f07 Mon Sep 17 00:00:00 2001 From: Joshua Peraza Date: Tue, 3 Mar 2020 09:05:35 -0800 Subject: [PATCH] linux: add fallback-modes for memfd_create Bug: chromium:1051354 Change-Id: I5dbbb3b264c09060429db199aa9f046c2f317c48 Reviewed-on: https://chromium-review.googlesource.com/c/crashpad/crashpad/+/2080651 Reviewed-by: Mark Mentovai --- .../cros_crash_report_exception_handler.cc | 2 +- util/file/file_io.h | 19 +++++-- util/file/file_io_posix.cc | 57 +++++++++++++++---- util/file/file_io_test.cc | 17 ++++++ util/file/file_writer.cc | 2 +- util/file/file_writer.h | 2 +- 6 files changed, 81 insertions(+), 18 deletions(-) diff --git a/handler/linux/cros_crash_report_exception_handler.cc b/handler/linux/cros_crash_report_exception_handler.cc index b394d61f..92bc93e0 100644 --- a/handler/linux/cros_crash_report_exception_handler.cc +++ b/handler/linux/cros_crash_report_exception_handler.cc @@ -225,7 +225,7 @@ bool CrosCrashReportExceptionHandler::HandleExceptionWithConnection( AddUserExtensionStreams(user_stream_data_sources_, snapshot, &minidump); FileWriter file_writer; - if (!file_writer.OpenMemfd(base::FilePath("/tmp/minidump"))) { + if (!file_writer.OpenMemfd(base::FilePath("minidump"))) { Metrics::ExceptionCaptureResult(Metrics::CaptureResult::kOpenMemfdFailed); return false; } diff --git a/util/file/file_io.h b/util/file/file_io.h index 3c956cc2..6fa0f96d 100644 --- a/util/file/file_io.h +++ b/util/file/file_io.h @@ -399,16 +399,27 @@ FileHandle LoggingOpenFileForWrite(const base::FilePath& path, FilePermissions permissions); #if defined(OS_LINUX) -//! \brief Wraps memfd_create(), logging an error if the operation fails. -//! Unlike other file open operations, this doesn't set `O_CLOEXEC`. +//! \brief Opens an in-memory file for input and output. //! -//! \return The newly opened FileHandle, or an invalid FileHandle on failure. +//! This function first attempts to open the file with `memfd_create()`. If +//! `memfd_create()` isn't supported by the kernel, this function next attempts +//! to open a file using `O_TMPFILE`. If `O_TMPFILE` isn't supported, this +//! function finally falls back to creating a file with a randomized name in +//! `/tmp` and immediately `unlink()`ing it. +//! +//! Unlike other file open operations, this function doesn't set `O_CLOEXEC`. +//! +//! \param name A name associated with the file. This name does not indicate any +//! exact path and may not be used at all, depending on the strategy used to +//! create the file. The name should not contain any '/' characters. +//! \return The newly opened FileHandle, or an invalid FileHandle on failure, +//! with a message logged. //! //! \sa ScopedFileHandle //! \sa LoggingOpenFileForRead //! \sa LoggingOpenFileForWrite //! \sa LoggingOpenFileForReadAndWrite -FileHandle LoggingOpenMemFileForWrite(const base::FilePath& path); +FileHandle LoggingOpenMemoryFileForReadAndWrite(const base::FilePath& name); #endif // OS_LINUX //! \brief Wraps OpenFileForReadAndWrite(), logging an error if the operation diff --git a/util/file/file_io_posix.cc b/util/file/file_io_posix.cc index b72a48eb..91b252a0 100644 --- a/util/file/file_io_posix.cc +++ b/util/file/file_io_posix.cc @@ -14,6 +14,7 @@ #include "util/file/file_io.h" +#include #include #include #include @@ -26,7 +27,9 @@ #include "base/files/file_path.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" +#include "base/strings/stringprintf.h" #include "build/build_config.h" +#include "util/misc/random_string.h" namespace crashpad { @@ -98,13 +101,6 @@ FileHandle OpenFileForOutput(int rdwr_or_wronly, flags, permissions == FilePermissions::kWorldReadable ? 0644 : 0600)); } - -#if defined(OS_LINUX) -FileHandle OpenMemFileForOutput(const base::FilePath& path) { - return HANDLE_EINTR(memfd_create(path.value().c_str(), 0)); -} -#endif - } // namespace namespace internal { @@ -157,10 +153,49 @@ FileHandle LoggingOpenFileForWrite(const base::FilePath& path, } #if defined(OS_LINUX) -FileHandle LoggingOpenMemFileForWrite(const base::FilePath& path) { - FileHandle fd = OpenMemFileForOutput(path); - PLOG_IF(ERROR, fd < 0) << "memfd_create " << path.value(); - return fd; +FileHandle LoggingOpenMemoryFileForReadAndWrite(const base::FilePath& name) { + DCHECK(name.value().find('/') == std::string::npos); + + int result = HANDLE_EINTR(memfd_create(name.value().c_str(), 0)); + if (result >= 0 || errno != ENOSYS) { + PLOG_IF(ERROR, result < 0) << "memfd_create"; + return result; + } + + const char* tmp = getenv("TMPDIR"); + tmp = tmp ? tmp : "/tmp"; + + result = HANDLE_EINTR(open(tmp, O_RDWR | O_EXCL | O_TMPFILE, 0600)); + if (result >= 0 || + // These are the expected possible error codes indicating that O_TMPFILE + // doesn't have kernel or filesystem support. O_TMPFILE was added in Linux + // 3.11. Experimentation confirms that at least Linux 2.6.29 and Linux + // 3.10 set errno to EISDIR. EOPNOTSUPP is returned when the filesystem + // doesn't support O_TMPFILE. The man pages also mention ENOENT as an + // error code to check, but the language implies it would only occur when + // |tmp| is also an invalid directory. EINVAL is mentioned as a possible + // error code for any invalid values in flags, but O_TMPFILE isn't + // mentioned explicitly in this context and hasn't been observed in + // practice. + (errno != EISDIR && errno != EOPNOTSUPP)) { + PLOG_IF(ERROR, result < 0) << "open"; + return result; + } + + std::string path = base::StringPrintf("%s/%s.%d.%s", + tmp, + name.value().c_str(), + getpid(), + RandomString().c_str()); + result = HANDLE_EINTR(open(path.c_str(), O_RDWR | O_CREAT | O_EXCL, 0600)); + if (result < 0) { + PLOG(ERROR) << "open"; + return result; + } + if (unlink(path.c_str()) != 0) { + PLOG(WARNING) << "unlink"; + } + return result; } #endif diff --git a/util/file/file_io_test.cc b/util/file/file_io_test.cc index 0fdd25c4..0efded8a 100644 --- a/util/file/file_io_test.cc +++ b/util/file/file_io_test.cc @@ -473,6 +473,23 @@ TEST(FileIO, LoggingOpenFileForReadAndWrite) { TestOpenFileForWrite(LoggingOpenFileForReadAndWrite); } +#if defined(OS_LINUX) +TEST(FileIO, LoggingOpenMemoryFileForReadAndWrite) { + ScopedFileHandle handle( + LoggingOpenMemoryFileForReadAndWrite(base::FilePath("memfile"))); + ASSERT_TRUE(handle.is_valid()); + + static constexpr char kTestData[] = "somedata"; + ASSERT_TRUE(LoggingWriteFile(handle.get(), kTestData, sizeof(kTestData))); + + ASSERT_EQ(LoggingSeekFile(handle.get(), 0, SEEK_SET), 0); + + char buffer[sizeof(kTestData)]; + ASSERT_TRUE(LoggingReadFileExactly(handle.get(), buffer, sizeof(buffer))); + EXPECT_EQ(memcmp(buffer, kTestData, sizeof(buffer)), 0); +} +#endif // OS_LINUX + enum class ReadOrWrite : bool { kRead, kWrite, diff --git a/util/file/file_writer.cc b/util/file/file_writer.cc index de28575d..6dff975a 100644 --- a/util/file/file_writer.cc +++ b/util/file/file_writer.cc @@ -174,7 +174,7 @@ bool FileWriter::Open(const base::FilePath& path, #if defined(OS_LINUX) bool FileWriter::OpenMemfd(const base::FilePath& path) { CHECK(!file_.is_valid()); - file_.reset(LoggingOpenMemFileForWrite(path)); + file_.reset(LoggingOpenMemoryFileForReadAndWrite(path)); if (!file_.is_valid()) { return false; } diff --git a/util/file/file_writer.h b/util/file/file_writer.h index 663adff1..4b99b375 100644 --- a/util/file/file_writer.h +++ b/util/file/file_writer.h @@ -132,7 +132,7 @@ class FileWriter : public FileWriterInterface { FilePermissions permissions); #if defined(OS_LINUX) - //! \brief Wraps LoggingOpenMemFileForWrite(). + //! \brief Wraps LoggingOpenMemoryFileForWrite(). //! //! \return `true` if the operation succeeded, `false` if it failed, with an //! error message logged.