From 5bd897d39d2a36399944f4d3c87f6a62455db2f7 Mon Sep 17 00:00:00 2001 From: Balazs Engedy Date: Wed, 9 Sep 2015 14:38:16 +0200 Subject: [PATCH] Revert "Add sbox tests related to warming up of locales." Speculative revert of https://codereview.chromium.org/1324523008. After that CL, multiple XP bots started failing in strange ways on content_browsertests, sync_integration_tests, and telemetry_tests. BUG=464430 TBR=liamjm@chromium.org, jochen@chromium.org, isherman@chromium.org, wfh@chromium.org Review URL: https://codereview.chromium.org/1328353002 . Cr-Commit-Position: refs/heads/master@{#347914} --- .../nacl/loader/nacl_main_platform_delegate_win.cc | 3 + .../renderer_main_platform_delegate_win.cc | 3 + sandbox/win/BUILD.gn | 1 - sandbox/win/sandbox_win.gypi | 1 - sandbox/win/src/lpc_policy_test.cc | 119 --------------------- sandbox/win/src/sandbox_policy.h | 2 +- sandbox/win/src/sandbox_types.h | 1 - sandbox/win/src/target_services.cc | 18 ---- tools/metrics/histograms/histograms.xml | 1 - 9 files changed, 7 insertions(+), 142 deletions(-) delete mode 100644 sandbox/win/src/lpc_policy_test.cc diff --git a/components/nacl/loader/nacl_main_platform_delegate_win.cc b/components/nacl/loader/nacl_main_platform_delegate_win.cc index c0caa5e2c2d2..e4d0ad5520d6 100644 --- a/components/nacl/loader/nacl_main_platform_delegate_win.cc +++ b/components/nacl/loader/nacl_main_platform_delegate_win.cc @@ -17,6 +17,9 @@ void NaClMainPlatformDelegate::EnableSandbox( // Cause advapi32 to load before the sandbox is turned on. unsigned int dummy_rand; rand_s(&dummy_rand); + // Warm up language subsystems before the sandbox is turned on. + ::GetUserDefaultLangID(); + ::GetUserDefaultLCID(); // Turn the sandbox on. target_services->LowerToken(); diff --git a/content/renderer/renderer_main_platform_delegate_win.cc b/content/renderer/renderer_main_platform_delegate_win.cc index ae5c05636bdb..c397206b6637 100644 --- a/content/renderer/renderer_main_platform_delegate_win.cc +++ b/content/renderer/renderer_main_platform_delegate_win.cc @@ -105,6 +105,9 @@ bool RendererMainPlatformDelegate::EnableSandbox() { // Cause advapi32 to load before the sandbox is turned on. unsigned int dummy_rand; rand_s(&dummy_rand); + // Warm up language subsystems before the sandbox is turned on. + ::GetUserDefaultLangID(); + ::GetUserDefaultLCID(); target_services->LowerToken(); return true; diff --git a/sandbox/win/BUILD.gn b/sandbox/win/BUILD.gn index 68f9ae5ac6ac..b830534bb231 100644 --- a/sandbox/win/BUILD.gn +++ b/sandbox/win/BUILD.gn @@ -196,7 +196,6 @@ test("sbox_integration_tests") { "src/handle_policy_test.cc", "src/integrity_level_test.cc", "src/ipc_ping_test.cc", - "src/lpc_policy_test.cc", "src/named_pipe_policy_test.cc", "src/policy_target_test.cc", "src/process_mitigations_test.cc", diff --git a/sandbox/win/sandbox_win.gypi b/sandbox/win/sandbox_win.gypi index 91e058a52b87..aeb8f03df71a 100644 --- a/sandbox/win/sandbox_win.gypi +++ b/sandbox/win/sandbox_win.gypi @@ -225,7 +225,6 @@ 'src/handle_closer_test.cc', 'src/integrity_level_test.cc', 'src/ipc_ping_test.cc', - 'src/lpc_policy_test.cc', 'src/named_pipe_policy_test.cc', 'src/policy_target_test.cc', 'src/process_mitigations_test.cc', diff --git a/sandbox/win/src/lpc_policy_test.cc b/sandbox/win/src/lpc_policy_test.cc deleted file mode 100644 index a39d67dc60b9..000000000000 --- a/sandbox/win/src/lpc_policy_test.cc +++ /dev/null @@ -1,119 +0,0 @@ -// Copyright (c) 2011 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. - -// These tests have been added to specifically tests issues arising from (A)LPC -// lock down. - -#include -#include - -#include -#include - -#include "sandbox/win/src/sandbox.h" -#include "sandbox/win/src/sandbox_factory.h" -#include "sandbox/win/src/sandbox_policy.h" -#include "sandbox/win/tests/common/controller.h" -#include "sandbox/win/tests/common/test_utils.h" -#include "testing/gtest/include/gtest/gtest.h" - -namespace sandbox { - -// Converts LCID to std::wstring for passing to sbox tests. -std::wstring LcidToWString(LCID lcid) { - wchar_t buff[10] = {0}; - int res = swprintf_s(buff, sizeof(buff)/sizeof(buff[0]), L"%08x", lcid); - if (-1 != res) { - return std::wstring(buff); - } - return std::wstring(); -} - -// Converts LANGID to std::wstring for passing to sbox tests. -std::wstring LangidToWString(LANGID langid) { - wchar_t buff[10] = {0}; - int res = swprintf_s(buff, sizeof(buff)/sizeof(buff[0]), L"%04x", langid); - if (-1 != res) { - return std::wstring(buff); - } - return std::wstring(); -} - -SBOX_TESTS_COMMAND int Lpc_GetUserDefaultLangID(int argc, wchar_t **argv) { - if (argc != 1) - return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; - std::wstring expected_langid_string(argv[0]); - - // This will cause an exception if not warmed up suitably. - LANGID langid = ::GetUserDefaultLangID(); - - std::wstring langid_string = LangidToWString(langid); - if (0 == wcsncmp(langid_string.c_str(), expected_langid_string.c_str(), 4)) { - return SBOX_TEST_SUCCEEDED; - } - return SBOX_TEST_FAILED; -} - -TEST(LpcPolicyTest, GetUserDefaultLangID) { - LANGID langid = ::GetUserDefaultLangID(); - std::wstring cmd = L"Lpc_GetUserDefaultLangID " + LangidToWString(langid); - TestRunner runner; - EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd.c_str())); -} - -SBOX_TESTS_COMMAND int Lpc_GetUserDefaultLCID(int argc, wchar_t **argv) { - if (argc != 1) - return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; - std::wstring expected_lcid_string(argv[0]); - - // This will cause an exception if not warmed up suitably. - LCID lcid = ::GetUserDefaultLCID(); - - std::wstring lcid_string = LcidToWString(lcid); - if (0 == wcsncmp(lcid_string.c_str(), expected_lcid_string.c_str(), 8)) { - return SBOX_TEST_SUCCEEDED; - } - return SBOX_TEST_FAILED; -} - -TEST(LpcPolicyTest, GetUserDefaultLCID) { - LCID lcid = ::GetUserDefaultLCID(); - std::wstring cmd = L"Lpc_GetUserDefaultLCID " + LcidToWString(lcid); - TestRunner runner; - EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd.c_str())); -} - -SBOX_TESTS_COMMAND int Lpc_GetUserDefaultLocaleName(int argc, wchar_t **argv) { - if (argc != 1) - return SBOX_TEST_FAILED_TO_EXECUTE_COMMAND; - std::wstring expected_locale_name(argv[0]); - - wchar_t locale_name[LOCALE_NAME_MAX_LENGTH] = { 0 }; - // This will cause an exception if not warmed up suitably. - int ret = ::GetUserDefaultLocaleName(locale_name, LOCALE_NAME_MAX_LENGTH); - if (!ret) { - return SBOX_TEST_FAILED; - } - if (!wcsnlen(locale_name, LOCALE_NAME_MAX_LENGTH)) { - return SBOX_TEST_FAILED; - } - if (0 == wcsncmp(locale_name, - expected_locale_name.c_str(), - LOCALE_NAME_MAX_LENGTH)) { - return SBOX_TEST_SUCCEEDED; - } - return SBOX_TEST_FAILED; -} - -TEST(LpcPolicyTest, GetUserDefaultLocaleName) { - wchar_t locale_name[LOCALE_NAME_MAX_LENGTH] = { 0 }; - int ret = ::GetUserDefaultLocaleName(locale_name, LOCALE_NAME_MAX_LENGTH); - EXPECT_NE(ret, 0); - std::wstring cmd = L"Lpc_GetUserDefaultLocaleName " + \ - std::wstring(locale_name); - TestRunner runner; - EXPECT_EQ(SBOX_TEST_SUCCEEDED, runner.RunTest(cmd.c_str())); -} - -} // namespace sandbox diff --git a/sandbox/win/src/sandbox_policy.h b/sandbox/win/src/sandbox_policy.h index 0c3e84738187..c1af8578866c 100644 --- a/sandbox/win/src/sandbox_policy.h +++ b/sandbox/win/src/sandbox_policy.h @@ -80,7 +80,7 @@ class TargetPolicy { // not compatible with AppContainer, see SetAppContainer. // lockdown: the security level for the token that comes into force after the // process calls TargetServices::LowerToken() or the process calls - // RevertToSelf(). See the explanation of each level in the TokenLevel + // ReverToSelf(). See the explanation of each level in the TokenLevel // definition. // Return value: SBOX_ALL_OK if the setting succeeds and false otherwise. // Returns false if the lockdown value is more permissive than the initial diff --git a/sandbox/win/src/sandbox_types.h b/sandbox/win/src/sandbox_types.h index f315b477a829..3e531be4f4f0 100644 --- a/sandbox/win/src/sandbox_types.h +++ b/sandbox/win/src/sandbox_types.h @@ -61,7 +61,6 @@ enum TerminationCodes { SBOX_FATAL_CLOSEHANDLES = 7010, // Failed to close pending handles. SBOX_FATAL_MITIGATION = 7011, // Could not set the mitigation policy. SBOX_FATAL_MEMORY_EXCEEDED = 7012, // Exceeded the job memory limit. - SBOX_FATAL_WARMUP = 7013, // Failed to warmup. SBOX_FATAL_LAST }; diff --git a/sandbox/win/src/target_services.cc b/sandbox/win/src/target_services.cc index fd16b9ea002d..116f0c929b81 100644 --- a/sandbox/win/src/target_services.cc +++ b/sandbox/win/src/target_services.cc @@ -59,22 +59,6 @@ bool CloseOpenHandles(bool* is_csrss_connected) { return true; } -// Warm up language subsystems before the sandbox is turned on. -// Tested on Win8.1 x64: -// This needs to happen after RevertToSelf() is called, because (at least) in -// the case of GetUserDefaultLCID() it checks the TEB to see if the process is -// impersonating (TEB!IsImpersonating). If it is, the cached locale information -// is not used, nor is it set. Therefore, calls after RevertToSelf() will not -// have warmed-up values to use. -bool WarmupWindowsLocales() { - // NOTE(liamjm): When last checked (Win 8.1 x64) it wasn't necessary to - // warmup all of these functions, but let's not assume that. - ::GetUserDefaultLangID(); - ::GetUserDefaultLCID(); - wchar_t localeName[LOCALE_NAME_MAX_LENGTH] = { 0 }; - return (0 != ::GetUserDefaultLocaleName( - localeName, LOCALE_NAME_MAX_LENGTH * sizeof(wchar_t))); -} // Used as storage for g_target_services, because other allocation facilities // are not available early. We can't use a regular function static because on @@ -113,8 +97,6 @@ void TargetServicesBase::LowerToken() { ::TerminateProcess(::GetCurrentProcess(), SBOX_FATAL_FLUSHANDLES); if (ERROR_SUCCESS != ::RegDisablePredefinedCache()) ::TerminateProcess(::GetCurrentProcess(), SBOX_FATAL_CACHEDISABLE); - if (!WarmupWindowsLocales()) - ::TerminateProcess(::GetCurrentProcess(), SBOX_FATAL_WARMUP); bool is_csrss_connected = true; if (!CloseOpenHandles(&is_csrss_connected)) ::TerminateProcess(::GetCurrentProcess(), SBOX_FATAL_CLOSEHANDLES); diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index 119916b2a9ca..896959cd4845 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml @@ -55098,7 +55098,6 @@ http://cs/file:chrome/histograms.xml - but prefer this file for new entries. - -- 2.11.4.GIT