From 2f2012818b3cb814dacbc2b77c6b8966cc939d97 Mon Sep 17 00:00:00 2001 From: Jed Davis Date: Mon, 13 Jul 2015 16:17:58 -0700 Subject: [PATCH] Bug 1181704 - Use chromium SafeSPrintf for sandbox logging. r=kang r=glandium This gives us a logging macro that's safe to use in async signal context (cf. bug 1046210, where we needed this and didn't have it). This patch also changes one of the format strings to work with SafeSPrintf's format string dialect; upstream would probably take a patch to handle those letters, but this is easier. --- security/sandbox/linux/Sandbox.cpp | 4 +- security/sandbox/linux/SandboxFilter.cpp | 1 + security/sandbox/linux/SandboxLogging.cpp | 63 +++++++++++++++++++++++ security/sandbox/linux/SandboxLogging.h | 46 ++++++++++++++--- security/sandbox/linux/glue/moz.build | 7 +++ security/sandbox/linux/moz.build | 7 +++ 6 files changed, 118 insertions(+), 10 deletions(-) create mode 100644 security/sandbox/linux/SandboxLogging.cpp diff --git a/security/sandbox/linux/Sandbox.cpp b/security/sandbox/linux/Sandbox.cpp index 2f851e25d9f..1046ccbbd68 100644 --- a/security/sandbox/linux/Sandbox.cpp +++ b/security/sandbox/linux/Sandbox.cpp @@ -140,8 +140,8 @@ SigSysHandler(int nr, siginfo_t *info, void *void_context) // TODO, someday when this is enabled on MIPS: include the two extra // args in the error message. - SANDBOX_LOG_ERROR("seccomp sandbox violation: pid %d, syscall %lu," - " args %lu %lu %lu %lu %lu %lu. Killing process.", + SANDBOX_LOG_ERROR("seccomp sandbox violation: pid %d, syscall %d," + " args %d %d %d %d %d %d. Killing process.", pid, syscall_nr, args[0], args[1], args[2], args[3], args[4], args[5]); diff --git a/security/sandbox/linux/SandboxFilter.cpp b/security/sandbox/linux/SandboxFilter.cpp index d237ae890ee..6afeaaa2b80 100644 --- a/security/sandbox/linux/SandboxFilter.cpp +++ b/security/sandbox/linux/SandboxFilter.cpp @@ -166,6 +166,7 @@ public: // Simple I/O case __NR_write: case __NR_read: + case __NR_writev: // see SandboxLogging.cpp return Allow(); // Memory mapping diff --git a/security/sandbox/linux/SandboxLogging.cpp b/security/sandbox/linux/SandboxLogging.cpp new file mode 100644 index 00000000000..952b5de4e03 --- /dev/null +++ b/security/sandbox/linux/SandboxLogging.cpp @@ -0,0 +1,63 @@ +/* -*- Mode: C++; tab-width: 8; indent-tabs-mode: nil; c-basic-offset: 2 -*- */ +/* vim: set ts=8 sts=2 et sw=2 tw=80: */ +/* This Source Code Form is subject to the terms of the Mozilla Public + * License, v. 2.0. If a copy of the MPL was not distributed with this file, + * You can obtain one at http://mozilla.org/MPL/2.0/. */ + +#include "SandboxLogging.h" + +#ifdef ANDROID +#include +#else +#include +#include +#include +#include +#endif + +#include "base/posix/eintr_wrapper.h" + +namespace mozilla { + +#ifndef ANDROID +// Alters an iovec array to remove the first `toDrop` bytes. This +// complexity is necessary because writev can return a short write +// (e.g., if stderr is a pipe and the buffer is almost full). +static void +IOVecDrop(struct iovec* iov, int iovcnt, size_t toDrop) +{ + while (toDrop > 0 && iovcnt > 0) { + size_t toDropHere = std::min(toDrop, iov->iov_len); + iov->iov_base = static_cast(iov->iov_base) + toDropHere; + iov->iov_len -= toDropHere; + toDrop -= toDropHere; + ++iov; + --iovcnt; + } +} +#endif + +void +SandboxLogError(const char* message) +{ +#ifdef ANDROID + // This uses writev internally and appears to be async signal safe. + __android_log_write(ANDROID_LOG_ERROR, "Sandbox", message); +#else + static const char logPrefix[] = "Sandbox: ", logSuffix[] = "\n"; + struct iovec iovs[3] = { + { const_cast(logPrefix), sizeof(logPrefix) - 1 }, + { const_cast(message), strlen(message) }, + { const_cast(logSuffix), sizeof(logSuffix) - 1 }, + }; + while (iovs[2].iov_len > 0) { + ssize_t written = HANDLE_EINTR(writev(STDERR_FILENO, iovs, 3)); + if (written <= 0) { + break; + } + IOVecDrop(iovs, 3, static_cast(written)); + } +#endif +} + +} diff --git a/security/sandbox/linux/SandboxLogging.h b/security/sandbox/linux/SandboxLogging.h index 0a6794a50ac..88891adfb12 100644 --- a/security/sandbox/linux/SandboxLogging.h +++ b/security/sandbox/linux/SandboxLogging.h @@ -7,16 +7,46 @@ #ifndef mozilla_SandboxLogging_h #define mozilla_SandboxLogging_h -#if defined(ANDROID) -#include +// This header defines the SANDBOX_LOG_ERROR macro used in the Linux +// sandboxing code. It uses Android logging on Android and writes to +// stderr otherwise. Android logging has severity levels; currently +// only "error" severity is exposed here, and this isn't marked when +// writing to stderr. +// +// The format strings are processed by Chromium SafeSPrintf, which +// doesn't accept size modifiers or %u because it uses C++11 variadic +// templates to obtain the actual argument types; all decimal integer +// formatting uses %d. See safe_sprintf.h for more details. + +// Build SafeSPrintf without assertions to avoid a dependency on +// Chromium logging. This doesn't affect safety; it just means that +// type mismatches (pointer vs. integer) always result in unexpanded +// %-directives instead of crashing. See also the moz.build files, +// which apply NDEBUG to the .cc file. +#ifndef NDEBUG +#define NDEBUG 1 +#include "base/strings/safe_sprintf.h" +#undef NDEBUG #else -#include +#include "base/strings/safe_sprintf.h" #endif -#if defined(ANDROID) -#define SANDBOX_LOG_ERROR(args...) __android_log_print(ANDROID_LOG_ERROR, "Sandbox", ## args) -#else -#define SANDBOX_LOG_ERROR(fmt, args...) fprintf(stderr, "Sandbox: " fmt "\n", ## args) -#endif +namespace mozilla { +// Logs the formatted string (marked with "error" severity, if supported). +void SandboxLogError(const char* aMessage); +} + +#define SANDBOX_LOG_LEN 256 + +// Formats a log message and logs it (with "error" severity, if supported). +// +// Note that SafeSPrintf doesn't accept size modifiers or %u; all +// decimal integers are %d, because it uses C++11 variadic templates +// to use the actual argument type. +#define SANDBOX_LOG_ERROR(fmt, args...) do { \ + char _sandboxLogBuf[SANDBOX_LOG_LEN]; \ + ::base::strings::SafeSPrintf(_sandboxLogBuf, fmt, ## args); \ + ::mozilla::SandboxLogError(_sandboxLogBuf); \ +} while(0) #endif // mozilla_SandboxLogging_h diff --git a/security/sandbox/linux/glue/moz.build b/security/sandbox/linux/glue/moz.build index 80f0fd15729..9e0c64e1bf0 100644 --- a/security/sandbox/linux/glue/moz.build +++ b/security/sandbox/linux/glue/moz.build @@ -7,10 +7,17 @@ FAIL_ON_WARNINGS = True SOURCES += [ + '../../chromium/base/strings/safe_sprintf.cc', + '../SandboxLogging.cpp', 'SandboxCrash.cpp', ] +# Avoid Chromium logging dependency, because this is going into +# libxul. See also the comment in SandboxLogging.h. +SOURCES['../../chromium/base/strings/safe_sprintf.cc'].flags += ['-DNDEBUG'] + LOCAL_INCLUDES += [ + '/security/sandbox/chromium', '/security/sandbox/linux', ] diff --git a/security/sandbox/linux/moz.build b/security/sandbox/linux/moz.build index 150090f8344..ce1a40fdcd8 100644 --- a/security/sandbox/linux/moz.build +++ b/security/sandbox/linux/moz.build @@ -25,6 +25,7 @@ SOURCES += [ '../chromium/base/lazy_instance.cc', '../chromium/base/memory/ref_counted.cc', '../chromium/base/memory/singleton.cc', + '../chromium/base/strings/safe_sprintf.cc', '../chromium/base/strings/string16.cc', '../chromium/base/strings/string_piece.cc', '../chromium/base/strings/string_util.cc', @@ -61,9 +62,15 @@ SOURCES += [ 'SandboxChroot.cpp', 'SandboxFilter.cpp', 'SandboxFilterUtil.cpp', + 'SandboxLogging.cpp', 'SandboxUtil.cpp', ] +# This copy of SafeSPrintf doesn't need to avoid the Chromium logging +# dependency like the one in libxul does, but this way the behavior is +# consistent. See also the comment in SandboxLogging.h. +SOURCES['../chromium/base/strings/safe_sprintf.cc'].flags += ['-DNDEBUG'] + # gcc lto likes to put the top level asm in syscall.cc in a different partition # from the function using it which breaks the build. Work around that by # forcing there to be only one partition.