Bug 1181704 - Use chromium SafeSPrintf for sandbox logging. r=kang r=glandium
☠☠ backed out by 201c980cabe7 ☠ ☠
authorJed Davis <jld@mozilla.com>
Mon, 13 Jul 2015 16:17:58 -0700
changeset 252661 fbf7aca43c3a79cabf6bc05adc80dc930cae43f3
parent 252660 8864c0587ced302104adadef96605907a861bf87
child 252662 18feee96dfb6f1c3cde31def2e870282ee2830dc
push id29042
push usercbook@mozilla.com
push dateTue, 14 Jul 2015 10:23:28 +0000
treeherdermozilla-central@e786406bc683 [default view] [failures only]
perfherder[talos] [build metrics] [platform microbench] (compared to previous push)
reviewerskang, glandium
bugs1181704, 1046210
milestone42.0a1
first release with
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
last release without
nightly linux32
nightly linux64
nightly mac
nightly win32
nightly win64
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
security/sandbox/linux/SandboxFilter.cpp
security/sandbox/linux/SandboxLogging.cpp
security/sandbox/linux/SandboxLogging.h
security/sandbox/linux/glue/moz.build
security/sandbox/linux/moz.build
--- a/security/sandbox/linux/Sandbox.cpp
+++ b/security/sandbox/linux/Sandbox.cpp
@@ -135,18 +135,18 @@ SigSysHandler(int nr, siginfo_t *info, v
   args[1] = SECCOMP_PARM2(&savedCtx);
   args[2] = SECCOMP_PARM3(&savedCtx);
   args[3] = SECCOMP_PARM4(&savedCtx);
   args[4] = SECCOMP_PARM5(&savedCtx);
   args[5] = SECCOMP_PARM6(&savedCtx);
 
   // 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]);
 
   // Bug 1017393: record syscall number somewhere useful.
   info->si_addr = reinterpret_cast<void*>(syscall_nr);
 
   gSandboxCrashFunc(nr, info, &savedCtx);
   _exit(127);
--- a/security/sandbox/linux/SandboxFilter.cpp
+++ b/security/sandbox/linux/SandboxFilter.cpp
@@ -161,16 +161,17 @@ public:
 
       // Metadata of opened files
     CASES_FOR_fstat:
       return Allow();
 
       // Simple I/O
     case __NR_write:
     case __NR_read:
+    case __NR_writev: // see SandboxLogging.cpp
       return Allow();
 
       // Memory mapping
     CASES_FOR_mmap:
     case __NR_munmap:
       return Allow();
 
       // Signal handling
new file mode 100644
--- /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 <android/log.h>
+#else
+#include <algorithm>
+#include <stdio.h>
+#include <sys/uio.h>
+#include <unistd.h>
+#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<char*>(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<char*>(logPrefix), sizeof(logPrefix) - 1 },
+    { const_cast<char*>(message), strlen(message) },
+    { const_cast<char*>(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<size_t>(written));
+  }
+#endif
+}
+
+}
--- a/security/sandbox/linux/SandboxLogging.h
+++ b/security/sandbox/linux/SandboxLogging.h
@@ -2,21 +2,51 @@
 /* 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/. */
 
 #ifndef mozilla_SandboxLogging_h
 #define mozilla_SandboxLogging_h
 
-#if defined(ANDROID)
-#include <android/log.h>
+// 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 <stdio.h>
+#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
--- a/security/sandbox/linux/glue/moz.build
+++ b/security/sandbox/linux/glue/moz.build
@@ -2,20 +2,27 @@
 # vim: set filetype=python:
 # 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/.
 
 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',
 ]
 
 if CONFIG['OS_TARGET'] == 'Android':
     USE_LIBS += [
         'mozsandbox',
     ]
 
--- a/security/sandbox/linux/moz.build
+++ b/security/sandbox/linux/moz.build
@@ -20,16 +20,17 @@ EXPORTS.mozilla += [
 
 SOURCES += [
     '../chromium-shim/base/logging.cpp',
     '../chromium/base/at_exit.cc',
     '../chromium/base/callback_internal.cc',
     '../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',
     '../chromium/base/strings/string_util_constants.cc',
     '../chromium/base/strings/stringprintf.cc',
     '../chromium/base/strings/utf_string_conversion_utils.cc',
     '../chromium/base/strings/utf_string_conversions.cc',
     '../chromium/base/synchronization/condition_variable_posix.cc',
@@ -56,19 +57,25 @@ SOURCES += [
     '../chromium/sandbox/linux/seccomp-bpf/syscall.cc',
     '../chromium/sandbox/linux/seccomp-bpf/syscall_iterator.cc',
     '../chromium/sandbox/linux/seccomp-bpf/trap.cc',
     'LinuxCapabilities.cpp',
     'Sandbox.cpp',
     '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.
 if '-flto' in CONFIG['OS_CXXFLAGS'] and not CONFIG['CLANG_CXX']:
     LDFLAGS += ['--param lto-partitions=1']
 
 DEFINES['NS_NO_XPCOM'] = True
 DISABLE_STL_WRAPPING = True