From 6a4cabb866f66d4128a97cdc6d9d08ce074f1247 Mon Sep 17 00:00:00 2001 From: Allan Sandfeld Jensen Date: Wed, 5 Apr 2017 14:08:31 +0200 Subject: BASELINE: Update Chromium to 57.0.2987.144 Change-Id: I29db402ff696c71a04c4dbaec822c2e53efe0267 Reviewed-by: Peter Varga --- chromium/sandbox/BUILD.gn | 8 ++ chromium/sandbox/features.gni | 16 ++++ chromium/sandbox/linux/BUILD.gn | 1 + chromium/sandbox/linux/bpf_dsl/bpf_dsl.h | 5 ++ chromium/sandbox/linux/bpf_dsl/bpf_dsl_forward.h | 8 -- chromium/sandbox/linux/bpf_dsl/bpf_dsl_impl.h | 1 - .../integration_tests/bpf_dsl_seccomp_unittest.cc | 16 +++- .../linux/seccomp-bpf-helpers/sigsys_handlers.cc | 89 ++++++++++++++++++++++ chromium/sandbox/linux/services/credentials.cc | 3 +- chromium/sandbox/win/src/broker_services.h | 2 - chromium/sandbox/win/src/internal_types.h | 2 +- chromium/sandbox/win/src/policy_target_test.cc | 9 +++ .../sandbox/win/src/process_mitigations_test.cc | 2 + chromium/sandbox/win/src/target_process.cc | 19 +---- chromium/sandbox/win/src/target_process.h | 1 - chromium/sandbox/win/src/win_utils.cc | 3 +- 16 files changed, 149 insertions(+), 36 deletions(-) create mode 100644 chromium/sandbox/features.gni (limited to 'chromium/sandbox') diff --git a/chromium/sandbox/BUILD.gn b/chromium/sandbox/BUILD.gn index 8ca3574e182..8c0405ed8ec 100644 --- a/chromium/sandbox/BUILD.gn +++ b/chromium/sandbox/BUILD.gn @@ -2,6 +2,9 @@ # Use of this source code is governed by a BSD-style license that can be # found in the LICENSE file. +import("//build/buildflag_header.gni") +import("//sandbox/features.gni") + # Meta-target that forwards to the proper platform one. group("sandbox") { if (is_win) { @@ -19,3 +22,8 @@ group("sandbox") { ] } } + +buildflag_header("sandbox_features") { + header = "sandbox_features.h" + flags = [ "USE_SECCOMP_BPF=$use_seccomp_bpf" ] +} diff --git a/chromium/sandbox/features.gni b/chromium/sandbox/features.gni new file mode 100644 index 00000000000..aa18c04f193 --- /dev/null +++ b/chromium/sandbox/features.gni @@ -0,0 +1,16 @@ +# Copyright 2016 The Chromium Authors. All rights reserved. +# Use of this source code is governed by a BSD-style license that can be +# found in the LICENSE file. + +import("//build/config/nacl/config.gni") + +# The seccomp-bpf sandbox is only supported on five architectures +# currently. +# Do not disable seccomp_bpf anywhere without talking to +# security@chromium.org! +use_seccomp_bpf = + (is_linux || is_android) && + (current_cpu == "x86" || current_cpu == "x64" || current_cpu == "arm" || + current_cpu == "arm64" || current_cpu == "mipsel") + +use_seccomp_bpf = use_seccomp_bpf || is_nacl_nonsfi diff --git a/chromium/sandbox/linux/BUILD.gn b/chromium/sandbox/linux/BUILD.gn index 1e6d7a1c813..3e98defa5c6 100644 --- a/chromium/sandbox/linux/BUILD.gn +++ b/chromium/sandbox/linux/BUILD.gn @@ -4,6 +4,7 @@ import("//build/config/features.gni") import("//build/config/nacl/config.gni") +import("//sandbox/features.gni") import("//testing/test.gni") if (is_android) { diff --git a/chromium/sandbox/linux/bpf_dsl/bpf_dsl.h b/chromium/sandbox/linux/bpf_dsl/bpf_dsl.h index 7f813442374..6f0dd4eb39c 100644 --- a/chromium/sandbox/linux/bpf_dsl/bpf_dsl.h +++ b/chromium/sandbox/linux/bpf_dsl/bpf_dsl.h @@ -76,6 +76,11 @@ namespace sandbox { namespace bpf_dsl { +template +class Caser; + +class Elser; + // ResultExpr is an opaque reference to an immutable result expression tree. using ResultExpr = std::shared_ptr; diff --git a/chromium/sandbox/linux/bpf_dsl/bpf_dsl_forward.h b/chromium/sandbox/linux/bpf_dsl/bpf_dsl_forward.h index 10477c9b311..af1b48b4078 100644 --- a/chromium/sandbox/linux/bpf_dsl/bpf_dsl_forward.h +++ b/chromium/sandbox/linux/bpf_dsl/bpf_dsl_forward.h @@ -24,14 +24,6 @@ class BoolExprImpl; using ResultExpr = std::shared_ptr; using BoolExpr = std::shared_ptr; -template -class Arg; - -class Elser; - -template -class Caser; - } // namespace bpf_dsl } // namespace sandbox diff --git a/chromium/sandbox/linux/bpf_dsl/bpf_dsl_impl.h b/chromium/sandbox/linux/bpf_dsl/bpf_dsl_impl.h index 35ff64f4986..f397321eddd 100644 --- a/chromium/sandbox/linux/bpf_dsl/bpf_dsl_impl.h +++ b/chromium/sandbox/linux/bpf_dsl/bpf_dsl_impl.h @@ -13,7 +13,6 @@ namespace sandbox { namespace bpf_dsl { -class ErrorCode; class PolicyCompiler; namespace internal { diff --git a/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc b/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc index ae35677de2b..fc0ecf2e326 100644 --- a/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc +++ b/chromium/sandbox/linux/integration_tests/bpf_dsl_seccomp_unittest.cc @@ -1877,15 +1877,23 @@ BPF_TEST_C(SandboxBPF, PthreadBitMask, PthreadPolicyBitMask) { #if defined(__aarch64__) #ifndef PTRACE_GETREGS +#if defined(__GLIBC__) +#define PTRACE_GETREGS static_cast(12) +#else #define PTRACE_GETREGS 12 -#endif -#endif +#endif // defined(__GLIBC__) +#endif // !defined(PTRACE_GETREGS) +#endif // defined(__aarch64__) #if defined(__aarch64__) #ifndef PTRACE_SETREGS +#if defined(__GLIBC__) +#define PTRACE_SETREGS static_cast(13) +#else #define PTRACE_SETREGS 13 -#endif -#endif +#endif // defined(__GLIBC__) +#endif // !defined(PTRACE_SETREGS) +#endif // defined(__aarch64__) // Changes the syscall to run for a child being sandboxed using seccomp-bpf with // PTRACE_O_TRACESECCOMP. Should only be called when the child is stopped on diff --git a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc index ff730180019..00e9085d639 100644 --- a/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc +++ b/chromium/sandbox/linux/seccomp-bpf-helpers/sigsys_handlers.cc @@ -11,6 +11,7 @@ #include #include +#include "base/debug/crash_logging.h" #include "base/logging.h" #include "base/posix/eintr_wrapper.h" #include "build/build_config.h" @@ -93,6 +94,7 @@ void PrintSyscallError(uint32_t sysno) { rem /= 10; sysno_base10[i] = '0' + mod; } + #if defined(__mips__) && (_MIPS_SIM == _MIPS_SIM_ABI32) static const char kSeccompErrorPrefix[] = __FILE__ ":**CRASHING**:" SECCOMP_MESSAGE_COMMON_CONTENT " in syscall 4000 + "; @@ -106,6 +108,87 @@ void PrintSyscallError(uint32_t sysno) { WriteToStdErr(kSeccompErrorPostfix, sizeof(kSeccompErrorPostfix) - 1); } +// Helper to convert a number of type T to a hexadecimal string using +// stack-allocated storage. +template +class NumberToHex { + public: + explicit NumberToHex(T value) { + static const char kHexChars[] = "0123456789abcdef"; + + memset(str_, '0', sizeof(str_)); + str_[1] = 'x'; + str_[sizeof(str_) - 1] = '\0'; + + T rem = value; + T mod = 0; + for (size_t i = sizeof(str_) - 2; i >= 2; --i) { + mod = rem % 16; + rem /= 16; + str_[i] = kHexChars[mod]; + } + } + + const char* str() const { return str_; } + + static size_t length() { return sizeof(str_) - 1; } + + private: + // HEX uses two characters per byte, with a leading '0x', and a trailing NUL. + char str_[sizeof(T) * 2 + 3]; +}; + +// Records the syscall number and first four arguments in a crash key, to help +// debug the failure. +void SetSeccompCrashKey(const struct sandbox::arch_seccomp_data& args) { +#if !defined(OS_NACL_NONSFI) + NumberToHex nr(args.nr); + NumberToHex arg1(args.args[0]); + NumberToHex arg2(args.args[1]); + NumberToHex arg3(args.args[2]); + NumberToHex arg4(args.args[3]); + + // In order to avoid calling into libc sprintf functions from an unsafe signal + // context, manually construct the crash key string. + const char* const prefixes[] = { + "nr=", + " arg1=", + " arg2=", + " arg3=", + " arg4=", + }; + const char* const values[] = { + nr.str(), + arg1.str(), + arg2.str(), + arg3.str(), + arg4.str(), + }; + + size_t crash_key_length = nr.length() + arg1.length() + arg2.length() + + arg3.length() + arg4.length(); + for (const auto& prefix : prefixes) { + crash_key_length += strlen(prefix); + } + ++crash_key_length; // For the trailing NUL byte. + + char crash_key[crash_key_length]; + memset(crash_key, '\0', crash_key_length); + + size_t offset = 0; + for (size_t i = 0; i < arraysize(values); ++i) { + const char* strings[2] = { prefixes[i], values[i] }; + for (const auto& string : strings) { + size_t string_len = strlen(string); + memmove(&crash_key[offset], string, string_len); + offset += string_len; + } + } + + base::debug::SetCrashKeyValue("seccomp-sigsys", crash_key); +#endif +} + } // namespace namespace sandbox { @@ -114,6 +197,7 @@ intptr_t CrashSIGSYS_Handler(const struct arch_seccomp_data& args, void* aux) { uint32_t syscall = SyscallNumberToOffsetFromBase(args.nr); PrintSyscallError(syscall); + SetSeccompCrashKey(args); // Encode 8-bits of the 1st two arguments too, so we can discern which socket // type, which fcntl, ... etc., without being likely to hit a mapped @@ -141,6 +225,7 @@ intptr_t SIGSYSCloneFailure(const struct arch_seccomp_data& args, void* aux) { static const char kSeccompCloneError[] = __FILE__":**CRASHING**:" SECCOMP_MESSAGE_CLONE_CONTENT "\n"; WriteToStdErr(kSeccompCloneError, sizeof(kSeccompCloneError) - 1); + SetSeccompCrashKey(args); // "flags" is the first argument in the kernel's clone(). // Mark as volatile to be able to find the value on the stack in a minidump. volatile uint64_t clone_flags = args.args[0]; @@ -161,6 +246,7 @@ intptr_t SIGSYSPrctlFailure(const struct arch_seccomp_data& args, static const char kSeccompPrctlError[] = __FILE__":**CRASHING**:" SECCOMP_MESSAGE_PRCTL_CONTENT "\n"; WriteToStdErr(kSeccompPrctlError, sizeof(kSeccompPrctlError) - 1); + SetSeccompCrashKey(args); // Mark as volatile to be able to find the value on the stack in a minidump. volatile uint64_t option = args.args[0]; volatile char* addr = @@ -175,6 +261,7 @@ intptr_t SIGSYSIoctlFailure(const struct arch_seccomp_data& args, static const char kSeccompIoctlError[] = __FILE__":**CRASHING**:" SECCOMP_MESSAGE_IOCTL_CONTENT "\n"; WriteToStdErr(kSeccompIoctlError, sizeof(kSeccompIoctlError) - 1); + SetSeccompCrashKey(args); // Make "request" volatile so that we can see it on the stack in a minidump. volatile uint64_t request = args.args[1]; volatile char* addr = reinterpret_cast(request & 0xFFFF); @@ -191,6 +278,7 @@ intptr_t SIGSYSKillFailure(const struct arch_seccomp_data& args, static const char kSeccompKillError[] = __FILE__":**CRASHING**:" SECCOMP_MESSAGE_KILL_CONTENT "\n"; WriteToStdErr(kSeccompKillError, sizeof(kSeccompKillError) - 1); + SetSeccompCrashKey(args); // Make "pid" volatile so that we can see it on the stack in a minidump. volatile uint64_t my_pid = sys_getpid(); volatile char* addr = reinterpret_cast(my_pid & 0xFFF); @@ -204,6 +292,7 @@ intptr_t SIGSYSFutexFailure(const struct arch_seccomp_data& args, static const char kSeccompFutexError[] = __FILE__ ":**CRASHING**:" SECCOMP_MESSAGE_FUTEX_CONTENT "\n"; WriteToStdErr(kSeccompFutexError, sizeof(kSeccompFutexError) - 1); + SetSeccompCrashKey(args); volatile int futex_op = args.args[1]; volatile char* addr = reinterpret_cast(futex_op & 0xFFF); *addr = '\0'; diff --git a/chromium/sandbox/linux/services/credentials.cc b/chromium/sandbox/linux/services/credentials.cc index 2265474104f..50a109e2f45 100644 --- a/chromium/sandbox/linux/services/credentials.cc +++ b/chromium/sandbox/linux/services/credentials.cc @@ -292,8 +292,7 @@ bool Credentials::CanCreateProcessInNewUserNS() { // Ensure we have unprivileged use of CLONE_NEWUSER. Debian // Jessie explicitly forbids this case. See: // add-sysctl-to-disallow-unprivileged-CLONE_NEWUSER-by-default.patch - PCHECK(0 == sys_unshare(CLONE_NEWUSER)); - _exit(kExitSuccess); + _exit(!!sys_unshare(CLONE_NEWUSER)); } // Always reap the child. diff --git a/chromium/sandbox/win/src/broker_services.h b/chromium/sandbox/win/src/broker_services.h index 6aad976b077..919478c4f8a 100644 --- a/chromium/sandbox/win/src/broker_services.h +++ b/chromium/sandbox/win/src/broker_services.h @@ -29,8 +29,6 @@ struct JobTracker; namespace sandbox { -class PolicyBase; - // BrokerServicesBase --------------------------------------------------------- // Broker implementation version 0 // diff --git a/chromium/sandbox/win/src/internal_types.h b/chromium/sandbox/win/src/internal_types.h index e1028189d80..7ea4b7d62ea 100644 --- a/chromium/sandbox/win/src/internal_types.h +++ b/chromium/sandbox/win/src/internal_types.h @@ -13,7 +13,7 @@ const wchar_t kNtdllName[] = L"ntdll.dll"; const wchar_t kKerneldllName[] = L"kernel32.dll"; const wchar_t kKernelBasedllName[] = L"kernelbase.dll"; -// Defines the supported C++ types encoding to numeric id. Like a poor's man +// Defines the supported C++ types encoding to numeric id. Like a simplified // RTTI. Note that true C++ RTTI will not work because the types are not // polymorphic anyway. enum ArgType { diff --git a/chromium/sandbox/win/src/policy_target_test.cc b/chromium/sandbox/win/src/policy_target_test.cc index e6a8ff3ed0e..61964f8197e 100644 --- a/chromium/sandbox/win/src/policy_target_test.cc +++ b/chromium/sandbox/win/src/policy_target_test.cc @@ -214,6 +214,15 @@ TEST(PolicyTargetTest, OpenProcess) { "Opens a process"; } +TEST(PolicyTargetTest, PolicyBaseNoJobLifetime) { + TestRunner runner(JOB_NONE, USER_RESTRICTED_SAME_ACCESS, USER_LOCKDOWN); + runner.SetReleasePolicyInRun(true); + // TargetPolicy and its SharedMemIPCServer should continue to exist until + // the child process dies. + EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(L"PolicyTargetTest_thread")) + << "Opens the current thread"; +} + // Launches the app in the sandbox and ask it to wait in an // infinite loop. Waits for 2 seconds and then check if the // desktop associated with the app thread is not the same as the diff --git a/chromium/sandbox/win/src/process_mitigations_test.cc b/chromium/sandbox/win/src/process_mitigations_test.cc index 7aae5964bec..53b0442c1c5 100644 --- a/chromium/sandbox/win/src/process_mitigations_test.cc +++ b/chromium/sandbox/win/src/process_mitigations_test.cc @@ -1536,6 +1536,8 @@ SBOX_TESTS_COMMAND int CheckWin10FontLoad(int argc, wchar_t** argv) { std::vector font_data; int64_t len = file.GetLength(); + if (len < 0) + return SBOX_TEST_NOT_FOUND; font_data.resize(len); int read = file.Read(0, &font_data[0], len); diff --git a/chromium/sandbox/win/src/target_process.cc b/chromium/sandbox/win/src/target_process.cc index 72e2780c8c3..d163cfb5504 100644 --- a/chromium/sandbox/win/src/target_process.cc +++ b/chromium/sandbox/win/src/target_process.cc @@ -63,30 +63,17 @@ TargetProcess::TargetProcess(base::win::ScopedHandle initial_token, base_address_(NULL) {} TargetProcess::~TargetProcess() { - DWORD exit_code = 0; // Give a chance to the process to die. In most cases the JOB_KILL_ON_CLOSE // will take effect only when the context changes. As far as the testing went, // this wait was enough to switch context and kill the processes in the job. // If this process is already dead, the function will return without waiting. - // TODO(nsylvain): If the process is still alive at the end, we should kill - // it. http://b/893891 // For now, this wait is there only to do a best effort to prevent some leaks // from showing up in purify. if (sandbox_process_info_.IsValid()) { ::WaitForSingleObject(sandbox_process_info_.process_handle(), 50); - // At this point, the target process should have been killed. Check. - if (!::GetExitCodeProcess(sandbox_process_info_.process_handle(), - &exit_code) || (STILL_ACTIVE == exit_code)) { - // Something went wrong. We don't know if the target is in a state where - // it can manage to do another IPC call. If it can, and we've destroyed - // the |ipc_server_|, it will crash the broker. So we intentionally leak - // that. - if (shared_section_.IsValid()) - shared_section_.Take(); - ignore_result(ipc_server_.release()); - sandbox_process_info_.TakeProcessHandle(); - return; - } + // Terminate the process if it's still alive, as its IPC server is going + // away. 1 is RESULT_CODE_KILLED. + ::TerminateProcess(sandbox_process_info_.process_handle(), 1); } // ipc_server_ references our process handle, so make sure the former is shut diff --git a/chromium/sandbox/win/src/target_process.h b/chromium/sandbox/win/src/target_process.h index 70b7b32c30e..7d469deccd8 100644 --- a/chromium/sandbox/win/src/target_process.h +++ b/chromium/sandbox/win/src/target_process.h @@ -28,7 +28,6 @@ class StartupInformation; namespace sandbox { -class AttributeList; class SharedMemIPCServer; class ThreadProvider; diff --git a/chromium/sandbox/win/src/win_utils.cc b/chromium/sandbox/win/src/win_utils.cc index 9dfb2c9fb92..51ae51d9fc0 100644 --- a/chromium/sandbox/win/src/win_utils.cc +++ b/chromium/sandbox/win/src/win_utils.cc @@ -489,7 +489,8 @@ void* GetProcessBaseAddress(HANDLE process) { next_base += mem_info.RegionSize; if (!next_base.IsValid()) return nullptr; - current = reinterpret_cast(next_base.ValueOrDie()); + current = + reinterpret_cast(static_cast(next_base.ValueOrDie())); } return nullptr; -- cgit v1.2.1