From e5890e1e8f97eb873972680e85e6c76ea3bc6923 Mon Sep 17 00:00:00 2001 From: "csharp@chromium.org" Date: Thu, 27 Feb 2014 17:15:05 +0000 Subject: [PATCH] Link chrome_elf.dll instead of statically linking the blacklist code This ensure that the dll code is referenced, instead of the target making its own copy. TBR=sky@chromium.org BUG=329023 Review URL: https://codereview.chromium.org/181373003 git-svn-id: svn://svn.chromium.org/chrome/trunk/src@253847 0039d316-1c4b-4281-b951-d872f2087c98 --- chrome/browser/DEPS | 2 +- .../browser/chrome_elf_init_unittest_win.cc | 2 +- chrome/browser/chrome_elf_init_win.cc | 6 +++- chrome/chrome_browser.gypi | 3 +- .../chrome_content_renderer_client.cc | 7 +---- chrome_elf/blacklist.gypi | 1 + chrome_elf/blacklist/blacklist.cc | 5 +--- chrome_elf/blacklist/blacklist.h | 24 ---------------- chrome_elf/blacklist/test/blacklist_test.cc | 1 + chrome_elf/chrome_elf.gyp | 23 ++++++++++++++- chrome_elf/chrome_elf_constants.cc | 7 +++++ chrome_elf/chrome_elf_constants.h | 28 +++++++++++++++++++ 12 files changed, 70 insertions(+), 39 deletions(-) diff --git a/chrome/browser/DEPS b/chrome/browser/DEPS index 9b4ece3fd304f5..313c8cec8d5fb8 100644 --- a/chrome/browser/DEPS +++ b/chrome/browser/DEPS @@ -3,7 +3,7 @@ include_rules = [ "+ash", "+chrome/app", "+chrome/installer", - "+chrome_elf/blacklist", + "+chrome_elf/chrome_elf_constants.h", "+chrome_elf/create_file", "+chromeos", "+components/autofill/content/browser", diff --git a/chrome/browser/chrome_elf_init_unittest_win.cc b/chrome/browser/chrome_elf_init_unittest_win.cc index 2cf68b48a343ca..513b96b883ba44 100644 --- a/chrome/browser/chrome_elf_init_unittest_win.cc +++ b/chrome/browser/chrome_elf_init_unittest_win.cc @@ -9,7 +9,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/test/test_reg_util_win.h" #include "chrome/common/chrome_version_info.h" -#include "chrome_elf/blacklist/blacklist.h" +#include "chrome_elf/chrome_elf_constants.h" #include "testing/gtest/include/gtest/gtest.h" class ChromeBlacklistTrialTest : public testing::Test { diff --git a/chrome/browser/chrome_elf_init_win.cc b/chrome/browser/chrome_elf_init_win.cc index 424f533c34090c..aa74c46f911fd3 100644 --- a/chrome/browser/chrome_elf_init_win.cc +++ b/chrome/browser/chrome_elf_init_win.cc @@ -7,7 +7,7 @@ #include "base/strings/utf_string_conversions.h" #include "base/win/registry.h" #include "chrome/browser/chrome_elf_init_win.h" -#include "chrome_elf/blacklist/blacklist.h" +#include "chrome_elf/chrome_elf_constants.h" #include "version.h" // NOLINT namespace { @@ -64,6 +64,10 @@ void BrowserBlacklistBeaconSetup() { blacklist::kRegistryBeaconPath, KEY_QUERY_VALUE | KEY_SET_VALUE); + // No point in trying to continue if the registry key isn't valid. + if (!blacklist_registry_key.Valid()) + return; + // Find the last recorded blacklist version. base::string16 blacklist_version; blacklist_registry_key.ReadValue(blacklist::kBeaconVersion, diff --git a/chrome/chrome_browser.gypi b/chrome/chrome_browser.gypi index d9c53291981869..f70568f5730fb9 100644 --- a/chrome/chrome_browser.gypi +++ b/chrome/chrome_browser.gypi @@ -3399,7 +3399,8 @@ 'chrome_process_finder', 'installer_util_strings', '../chrome/chrome.gyp:chrome_version_header', - '../chrome_elf/chrome_elf.gyp:blacklist', + '../chrome_elf/chrome_elf.gyp:chrome_elf', + '../chrome_elf/chrome_elf.gyp:chrome_elf_constants', '../google_update/google_update.gyp:google_update', '../third_party/iaccessible2/iaccessible2.gyp:iaccessible2', '../third_party/isimpledom/isimpledom.gyp:isimpledom', diff --git a/chrome/renderer/chrome_content_renderer_client.cc b/chrome/renderer/chrome_content_renderer_client.cc index 1ab1cc48525606..e0a237b0304904 100644 --- a/chrome/renderer/chrome_content_renderer_client.cc +++ b/chrome/renderer/chrome_content_renderer_client.cc @@ -365,12 +365,7 @@ void ChromeContentRendererClient::RenderThreadStarted() { // Report if the renderer process has been patched by chrome_elf. // TODO(csharp): Remove once the renderer is no longer getting // patched this way. - typedef bool(*IsBlacklistInitializedFunc)(); - IsBlacklistInitializedFunc is_blacklist_initialized = - reinterpret_cast( - GetProcAddress(GetModuleHandle(L"chrome_elf.dll"), - "IsBlacklistInitialized")); - if (is_blacklist_initialized && is_blacklist_initialized()) + if (blacklist::IsBlacklistInitialized()) UMA_HISTOGRAM_BOOLEAN("Blacklist.PatchedInRenderer", true); #endif } diff --git a/chrome_elf/blacklist.gypi b/chrome_elf/blacklist.gypi index 24fcec6232527f..86c97c959bafbb 100644 --- a/chrome_elf/blacklist.gypi +++ b/chrome_elf/blacklist.gypi @@ -23,6 +23,7 @@ '../base/base.gyp:base_static', '../chrome/chrome.gyp:chrome_version_header', '../chrome_elf/chrome_elf.gyp:chrome_elf_breakpad', + '../chrome_elf/chrome_elf.gyp:chrome_elf_constants', '../sandbox/sandbox.gyp:sandbox', ], }, diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc index 958d7103b798ba..73383d5395f30b 100644 --- a/chrome_elf/blacklist/blacklist.cc +++ b/chrome_elf/blacklist/blacklist.cc @@ -8,6 +8,7 @@ #include "base/basictypes.h" #include "chrome_elf/blacklist/blacklist_interceptions.h" +#include "chrome_elf/chrome_elf_constants.h" #include "sandbox/win/src/interception_internal.h" #include "sandbox/win/src/internal_types.h" #include "sandbox/win/src/sandbox_utils.h" @@ -28,10 +29,6 @@ const wchar_t* g_troublesome_dlls[kTroublesomeDllsMaxCount] = { NULL, }; -const wchar_t kRegistryBeaconPath[] = L"SOFTWARE\\Google\\Chrome\\BLBeacon"; -const wchar_t kBeaconVersion[] = L"version"; -const wchar_t kBeaconState[] = L"state"; - } // namespace blacklist // Allocate storage for thunks in a page of this module to save on doing diff --git a/chrome_elf/blacklist/blacklist.h b/chrome_elf/blacklist/blacklist.h index 2e21f20793e296..779525f5bccb52 100644 --- a/chrome_elf/blacklist/blacklist.h +++ b/chrome_elf/blacklist/blacklist.h @@ -17,30 +17,6 @@ const int kTroublesomeDllsMaxCount = 64; // The DLL blacklist. extern const wchar_t* g_troublesome_dlls[kTroublesomeDllsMaxCount]; -// The registry path of the blacklist beacon. -extern const wchar_t kRegistryBeaconPath[]; - -// The properties for the blacklist beacon. -extern const wchar_t kBeaconVersion[]; -extern const wchar_t kBeaconState[]; - -// The states for the blacklist setup code. -enum BlacklistState { - BLACKLIST_DISABLED = 0, - BLACKLIST_ENABLED, - // The blacklist setup code is running. If this is still set at startup, - // it means the last setup crashed. - BLACKLIST_SETUP_RUNNING, - // The blacklist thunk setup code is running. If this is still set at startup, - // it means the last setup crashed during thunk setup. - BLACKLIST_THUNK_SETUP, - // The blacklist code is currently intercepting MapViewOfSection. If this is - // still set at startup, it means we crashed during interception. - BLACKLIST_INTERCEPTING, - // Always keep this at the end. - BLACKLIST_STATE_MAX, -}; - #if defined(_WIN64) extern NtMapViewOfSectionFunction g_nt_map_view_of_section_func; #endif diff --git a/chrome_elf/blacklist/test/blacklist_test.cc b/chrome_elf/blacklist/test/blacklist_test.cc index 39db737138c285..096502d7174645 100644 --- a/chrome_elf/blacklist/test/blacklist_test.cc +++ b/chrome_elf/blacklist/test/blacklist_test.cc @@ -15,6 +15,7 @@ #include "base/win/registry.h" #include "chrome_elf/blacklist/blacklist.h" #include "chrome_elf/blacklist/test/blacklist_test_main_dll.h" +#include "chrome_elf/chrome_elf_constants.h" #include "testing/gtest/include/gtest/gtest.h" #include "version.h" // NOLINT diff --git a/chrome_elf/chrome_elf.gyp b/chrome_elf/chrome_elf.gyp index fa6ed36f773d49..5820fc277457d3 100644 --- a/chrome_elf/chrome_elf.gyp +++ b/chrome_elf/chrome_elf.gyp @@ -126,7 +126,7 @@ ], }, { - 'target_name': 'chrome_elf_common', + 'target_name': 'chrome_elf_constants', 'type': 'static_library', 'include_dirs': [ '..', @@ -134,6 +134,27 @@ 'sources': [ 'chrome_elf_constants.cc', 'chrome_elf_constants.h', + ], + 'conditions': [ + ['component=="shared_library"', { + # In component builds, all targets depend on chrome_redirects by + # default. Remove it here so we are able to test it. + 'dependencies!': [ + '../chrome_elf/chrome_elf.gyp:chrome_redirects', + ], + }], + ], + }, + { + 'target_name': 'chrome_elf_common', + 'type': 'static_library', + 'dependencies': [ + 'chrome_elf_constants', + ], + 'include_dirs': [ + '..', + ], + 'sources': [ 'chrome_elf_types.h', 'chrome_elf_util.cc', 'chrome_elf_util.h', diff --git a/chrome_elf/chrome_elf_constants.cc b/chrome_elf/chrome_elf_constants.cc index cd6076300cc746..8879da7db83b80 100644 --- a/chrome_elf/chrome_elf_constants.cc +++ b/chrome_elf/chrome_elf_constants.cc @@ -14,3 +14,10 @@ const wchar_t kLocalStateFilename[] = L"Local State"; const wchar_t kPreferencesFilename[] = L"Preferences"; const wchar_t kUserDataDirName[] = L"User Data"; +namespace blacklist { + +const wchar_t kRegistryBeaconPath[] = L"SOFTWARE\\Google\\Chrome\\BLBeacon"; +const wchar_t kBeaconVersion[] = L"version"; +const wchar_t kBeaconState[] = L"state"; + +} // namespace blacklist diff --git a/chrome_elf/chrome_elf_constants.h b/chrome_elf/chrome_elf_constants.h index fcfbc259770a72..29418153b4a194 100644 --- a/chrome_elf/chrome_elf_constants.h +++ b/chrome_elf/chrome_elf_constants.h @@ -14,4 +14,32 @@ extern const wchar_t kLocalStateFilename[]; extern const wchar_t kPreferencesFilename[]; extern const wchar_t kUserDataDirName[]; +namespace blacklist { + +// The registry path of the blacklist beacon. +extern const wchar_t kRegistryBeaconPath[]; + +// The properties for the blacklist beacon. +extern const wchar_t kBeaconVersion[]; +extern const wchar_t kBeaconState[]; + +// The states for the blacklist setup code. +enum BlacklistState { + BLACKLIST_DISABLED = 0, + BLACKLIST_ENABLED, + // The blacklist setup code is running. If this is still set at startup, + // it means the last setup crashed. + BLACKLIST_SETUP_RUNNING, + // The blacklist thunk setup code is running. If this is still set at startup, + // it means the last setup crashed during thunk setup. + BLACKLIST_THUNK_SETUP, + // The blacklist code is currently intercepting MapViewOfSection. If this is + // still set at startup, it means we crashed during interception. + BLACKLIST_INTERCEPTING, + // Always keep this at the end. + BLACKLIST_STATE_MAX, +}; + +} // namespace blacklist + #endif // CHROME_ELF_CHROME_ELF_CONSTANTS_H_