Add UMA stats to record when DLLs are successfully blocked in the Browser.

BUG=345287

Review URL: https://codereview.chromium.org/174013007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@254480 0039d316-1c4b-4281-b951-d872f2087c98

12 files changed, 210 insertions, 14 deletions

diff --git a/chrome_elf/blacklist/blacklist.cc b/chrome_elf/blacklist/blacklist.cc
index 5ce3773..681427a 100644
--- a/chrome_elf/blacklist/blacklist.cc
+++ b/chrome_elf/blacklist/blacklist.cc
@@ -4,6 +4,7 @@
 
 #include "chrome_elf/blacklist/blacklist.h"
 
+#include <assert.h>
 #include <string.h>
 
 #include "base/basictypes.h"
@@ -30,6 +31,9 @@ const wchar_t* g_troublesome_dlls[kTroublesomeDllsMaxCount] = {
   NULL,
 };
 
+bool g_blocked_dlls[kTroublesomeDllsMaxCount] = {};
+int g_num_blocked_dlls = 0;
+
 }  // namespace blacklist
 
 // Allocate storage for thunks in a page of this module to save on doing
@@ -285,6 +289,7 @@ bool AddDllToBlacklist(const wchar_t* dll_name) {
   wcscpy(str_buffer, dll_name);
 
   g_troublesome_dlls[blacklist_size] = str_buffer;
+  g_blocked_dlls[blacklist_size] = false;
   return true;
 }
 
@@ -297,12 +302,52 @@ bool RemoveDllFromBlacklist(const wchar_t* dll_name) {
       delete[] g_troublesome_dlls[i];
       g_troublesome_dlls[i] = g_troublesome_dlls[blacklist_size - 1];
       g_troublesome_dlls[blacklist_size - 1] = NULL;
+
+      // Also update the stats recording if we have blocked this dll or not.
+      if (g_blocked_dlls[i])
+        --g_num_blocked_dlls;
+      g_blocked_dlls[i] = g_blocked_dlls[blacklist_size - 1];
       return true;
     }
   }
   return false;
 }
 
+// TODO(csharp): Maybe store these values in the registry so we can
+// still report them if Chrome crashes early.
+void SuccessfullyBlocked(const wchar_t** blocked_dlls, int* size) {
+  if (size == NULL)
+    return;
+
+  // If the array isn't valid or big enough, just report the size it needs to
+  // be and return.
+  if (blocked_dlls == NULL && *size < g_num_blocked_dlls) {
+    *size = g_num_blocked_dlls;
+    return;
+  }
+
+  *size = g_num_blocked_dlls;
+
+  int strings_to_fill = 0;
+  for (int i = 0; strings_to_fill < g_num_blocked_dlls && g_troublesome_dlls[i];
+       ++i) {
+    if (g_blocked_dlls[i]) {
+      blocked_dlls[strings_to_fill] = g_troublesome_dlls[i];
+      ++strings_to_fill;
+    }
+  }
+}
+
+void BlockedDll(size_t blocked_index) {
+  assert(blocked_index < kTroublesomeDllsMaxCount);
+
+  if (!g_blocked_dlls[blocked_index] &&
+      blocked_index < kTroublesomeDllsMaxCount) {
+    ++g_num_blocked_dlls;
+    g_blocked_dlls[blocked_index] = true;
+  }
+}
+
 bool Initialize(bool force) {
   // Check to see that we found the functions we need in ntdll.
   if (!InitializeInterceptImports())
diff --git a/chrome_elf/blacklist/blacklist.h b/chrome_elf/blacklist/blacklist.h
index 779525f..9877c29 100644
--- a/chrome_elf/blacklist/blacklist.h
+++ b/chrome_elf/blacklist/blacklist.h
@@ -12,7 +12,7 @@
 namespace blacklist {
 
 // Max size of the DLL blacklist.
-const int kTroublesomeDllsMaxCount = 64;
+const size_t kTroublesomeDllsMaxCount = 64;
 
 // The DLL blacklist.
 extern const wchar_t* g_troublesome_dlls[kTroublesomeDllsMaxCount];
@@ -44,12 +44,25 @@ extern "C" bool IsBlacklistInitialized();
 // the blacklist when this returns, false on error. Note that this will copy
 // |dll_name| and will leak it on exit if the string is not subsequently removed
 // using RemoveDllFromBlacklist.
+// Exposed for testing only, this shouldn't be exported from chrome_elf.dll.
 extern "C" bool AddDllToBlacklist(const wchar_t* dll_name);
 
 // Removes the given dll name from the blacklist. Returns true if it was
 // removed, false on error.
+// Exposed for testing only, this shouldn't be exported from chrome_elf.dll.
 extern "C" bool RemoveDllFromBlacklist(const wchar_t* dll_name);
 
+// Returns a list of all the dlls that have been successfully blocked by the
+// blacklist via blocked_dlls, if there is enough space (according to |size|).
+// |size| will always be modified to be the number of dlls that were blocked.
+// The caller doesn't own the strings and isn't expected to free them. These
+// strings won't be hanging unless RemoveDllFromBlacklist is called, but it
+// is only exposed in tests (and should stay that way).
+extern "C" void SuccessfullyBlocked(const wchar_t** blocked_dlls, int* size);
+
+// Record that the dll at the given index was blocked.
+void BlockedDll(size_t blocked_index);
+
 // Initializes the DLL blacklist in the current process. This should be called
 // before any undesirable DLLs might be loaded. If |force| is set to true, then
 // initialization will take place even if a beacon is present. This is useful
diff --git a/chrome_elf/blacklist/blacklist_interceptions.cc b/chrome_elf/blacklist/blacklist_interceptions.cc
index b3f3d82..8496c48 100644
--- a/chrome_elf/blacklist/blacklist_interceptions.cc
+++ b/chrome_elf/blacklist/blacklist_interceptions.cc
@@ -35,12 +35,12 @@ FARPROC GetNtDllExportByName(const char* export_name) {
   return ::GetProcAddress(ntdll, export_name);
 }
 
-bool DllMatch(const base::string16& module_name) {
+int DllMatch(const base::string16& module_name) {
   for (int i = 0; blacklist::g_troublesome_dlls[i] != NULL; ++i) {
     if (_wcsicmp(module_name.c_str(), blacklist::g_troublesome_dlls[i]) == 0)
-      return true;
+      return i;
   }
-  return false;
+  return -1;
 }
 
 // TODO(robertshield): Some of the helper functions below overlap somewhat with
@@ -203,10 +203,15 @@ NTSTATUS BlNtMapViewOfSectionImpl(
       module_name = ExtractLoadedModuleName(file_name);
     }
 
-    if (!module_name.empty() && DllMatch(module_name)) {
-      DCHECK_NT(g_nt_unmap_view_of_section_func);
-      g_nt_unmap_view_of_section_func(process, *base);
-      ret = STATUS_UNSUCCESSFUL;
+    if (!module_name.empty()) {
+      int blocked_index = DllMatch(module_name);
+      if (blocked_index != -1) {
+        DCHECK_NT(g_nt_unmap_view_of_section_func);
+        g_nt_unmap_view_of_section_func(process, *base);
+        ret = STATUS_UNSUCCESSFUL;
+
+        blacklist::BlockedDll(blocked_index);
+      }
     }
   }
 
diff --git a/chrome_elf/blacklist/test/blacklist_test.cc b/chrome_elf/blacklist/test/blacklist_test.cc
index 096502d..e507446 100644
--- a/chrome_elf/blacklist/test/blacklist_test.cc
+++ b/chrome_elf/blacklist/test/blacklist_test.cc
@@ -33,9 +33,12 @@ extern "C" {
 // functions on the test blacklist dll, not the ones linked into the test
 // executable itself.
 __declspec(dllimport) bool TestDll_AddDllToBlacklist(const wchar_t* dll_name);
-__declspec(dllimport) bool TestDLL_IsBlacklistInitialized();
+__declspec(dllimport) bool TestDll_IsBlacklistInitialized();
 __declspec(dllimport) bool TestDll_RemoveDllFromBlacklist(
     const wchar_t* dll_name);
+__declspec(dllimport) bool TestDll_SuccessfullyBlocked(
+    const wchar_t** blocked_dlls,
+    int* size);
 }
 
 class BlacklistTest : public testing::Test {
@@ -118,18 +121,50 @@ TEST_F(BlacklistTest, AddAndRemoveModules) {
     added_dlls[empty_spaces - 1].c_str()));
 }
 
+TEST_F(BlacklistTest, SuccessfullyBlocked) {
+  // Ensure that we have at least 5 dlls to blacklist.
+  int blacklist_size = blacklist::BlacklistSize();
+  const int kDesiredBlacklistSize = 5;
+  for (int i = blacklist_size; i < kDesiredBlacklistSize; ++i) {
+    base::string16 new_dll_name(base::IntToString16(i) + L".dll");
+    EXPECT_TRUE(blacklist::AddDllToBlacklist(new_dll_name.c_str()));
+  }
+
+  // Block 5 dlls, one at a time, starting from the end of the list, and
+  // ensuring SuccesfullyBlocked correctly passes the list of blocked dlls.
+  for (int i = 0; i < kDesiredBlacklistSize; ++i) {
+    blacklist::BlockedDll(i);
+
+    int size = 0;
+    blacklist::SuccessfullyBlocked(NULL, &size);
+    EXPECT_EQ(i + 1, size);
+
+    std::vector<const wchar_t*> blocked_dlls(size);
+    blacklist::SuccessfullyBlocked(&(blocked_dlls[0]), &size);
+    EXPECT_EQ(i + 1, size);
+
+    for (size_t j = 0; j < blocked_dlls.size(); ++j) {
+      EXPECT_EQ(blocked_dlls[j], blacklist::g_troublesome_dlls[j]);
+    }
+  }
+}
+
 TEST_F(BlacklistTest, LoadBlacklistedLibrary) {
   base::FilePath current_dir;
   ASSERT_TRUE(PathService::Get(base::DIR_EXE, &current_dir));
 
   // Ensure that the blacklist is loaded.
-  ASSERT_TRUE(TestDLL_IsBlacklistInitialized());
+  ASSERT_TRUE(TestDll_IsBlacklistInitialized());
 
   // Test that an un-blacklisted DLL can load correctly.
   base::ScopedNativeLibrary dll1(current_dir.Append(kTestDllName1));
   EXPECT_TRUE(dll1.is_valid());
   dll1.Reset(NULL);
 
+  int num_blocked_dlls = 0;
+  TestDll_SuccessfullyBlocked(NULL, &num_blocked_dlls);
+  EXPECT_EQ(0, num_blocked_dlls);
+
   struct TestData {
     const wchar_t* dll_name;
     const wchar_t* dll_beacon;
@@ -148,6 +183,13 @@ TEST_F(BlacklistTest, LoadBlacklistedLibrary) {
     EXPECT_EQ(0u, ::GetEnvironmentVariable(test_data[i].dll_beacon, NULL, 0));
     dll_blacklisted.Reset(NULL);
 
+    // Ensure that the dll is recorded as blocked.
+    int array_size = 1;
+    const wchar_t* blocked_dll = NULL;
+    TestDll_SuccessfullyBlocked(&blocked_dll, &array_size);
+    EXPECT_EQ(1, array_size);
+    EXPECT_EQ(test_data[i].dll_name, base::string16(blocked_dll));
+
     // Remove the DLL from the blacklist. Ensure that it loads and that its
     // entry point was called.
     EXPECT_TRUE(TestDll_RemoveDllFromBlacklist(test_data[i].dll_name));
@@ -169,5 +211,10 @@ TEST_F(BlacklistTest, LoadBlacklistedLibrary) {
     dll_blacklisted_different_case.Reset(NULL);
 
     EXPECT_TRUE(TestDll_RemoveDllFromBlacklist(uppercase_name.c_str()));
+
+    // The blocked dll was removed, so we shouldn't get anything returned
+    // here.
+    TestDll_SuccessfullyBlocked(NULL, &num_blocked_dlls);
+    EXPECT_EQ(0, num_blocked_dlls);
   }
 }
diff --git a/chrome_elf/blacklist/test/blacklist_test_main_dll.def b/chrome_elf/blacklist/test/blacklist_test_main_dll.def
index 82e0f0e..920dc6c 100644
--- a/chrome_elf/blacklist/test/blacklist_test_main_dll.def
+++ b/chrome_elf/blacklist/test/blacklist_test_main_dll.def
@@ -6,6 +6,7 @@ LIBRARY  "blacklist_test_main_dll.dll"
 
 EXPORTS
   TestDll_AddDllToBlacklist=AddDllToBlacklist
-  TestDLL_IsBlacklistInitialized=IsBlacklistInitialized
+  TestDll_IsBlacklistInitialized=IsBlacklistInitialized
+  TestDll_SuccessfullyBlocked=SuccessfullyBlocked
   TestDll_RemoveDllFromBlacklist=RemoveDllFromBlacklist
   InitBlacklistTestDll
\ No newline at end of file
diff --git a/chrome_elf/chrome_elf.def b/chrome_elf/chrome_elf.def
index 026f9c6..566a8fb 100644
--- a/chrome_elf/chrome_elf.def
+++ b/chrome_elf/chrome_elf.def
@@ -9,3 +9,4 @@ EXPORTS
   GetRedirectCount
   IsBlacklistInitialized
   SignalChromeElf
+  SuccessfullyBlocked
diff --git a/chrome_elf/chrome_elf.gyp b/chrome_elf/chrome_elf.gyp
index e564aa76..1534dbf 100644
--- a/chrome_elf/chrome_elf.gyp
+++ b/chrome_elf/chrome_elf.gyp
@@ -9,6 +9,7 @@
     '../build/util/version.gypi',
     '../build/win_precompile.gypi',
     'blacklist.gypi',
+    'dll_hash.gypi',
   ],
   'targets': [
     {
diff --git a/chrome_elf/chrome_elf_main.cc b/chrome_elf/chrome_elf_main.cc
index de64375..989493a 100644
--- a/chrome_elf/chrome_elf_main.cc
+++ b/chrome_elf/chrome_elf_main.cc
@@ -23,9 +23,6 @@ BOOL APIENTRY DllMain(HMODULE module, DWORD reason, LPVOID reserved) {
       blacklist::Initialize(false);  // Don't force, abort if beacon is present.
     } __except(GenerateCrashDump(GetExceptionInformation())) {
     }
-
-    // TODO(csharp): Move additions to the DLL blacklist to a sane place.
-    // blacklist::AddDllToBlacklist(L"foo.dll");
   }
 
   return TRUE;
diff --git a/chrome_elf/dll_hash.gypi b/chrome_elf/dll_hash.gypi
new file mode 100644
index 0000000..9e89a24
--- /dev/null
+++ b/chrome_elf/dll_hash.gypi
@@ -0,0 +1,31 @@
+# Copyright 2014 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.
+{
+  'targets': [
+    {
+      'target_name': 'dll_hash',
+      'type': 'static_library',
+      'dependencies': [
+        '../base/base.gyp:base',
+      ],
+      'sources': [
+        'dll_hash/dll_hash.cc',
+        'dll_hash/dll_hash.h',
+      ],
+    },
+    {
+      'target_name': 'dll_hash_main',
+      'type': 'executable',
+      'dependencies': [
+        'dll_hash',
+      ],
+      'include_dirs': [
+        '..',
+      ],
+      'sources': [
+        'dll_hash/dll_hash_main.cc',
+      ],
+    }
+  ]
+}
\ No newline at end of file
diff --git a/chrome_elf/dll_hash/dll_hash.cc b/chrome_elf/dll_hash/dll_hash.cc
new file mode 100644
index 0000000..ecd1491
--- /dev/null
+++ b/chrome_elf/dll_hash/dll_hash.cc
@@ -0,0 +1,14 @@
+// Copyright 2014 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.
+
+#include "base/hash.h"
+#include "chrome_elf/dll_hash/dll_hash.h"
+
+int DllNameToHash(std::string dll_name) {
+  uint32 data = base::Hash(dll_name);
+
+  // Strip off the signed bit because UMA doesn't support negative values,
+  // but takes a signed int as input.
+  return static_cast<int>(data & 0x7fffffff);
+}
diff --git a/chrome_elf/dll_hash/dll_hash.h b/chrome_elf/dll_hash/dll_hash.h
new file mode 100644
index 0000000..82bec8a
--- /dev/null
+++ b/chrome_elf/dll_hash/dll_hash.h
@@ -0,0 +1,13 @@
+// Copyright 2014 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.
+
+#ifndef CHROME_ELF_DLL_HASH_DLL_HASH_H_
+#define CHROME_ELF_DLL_HASH_DLL_HASH_H_
+
+#include <string>
+
+// Convert a dll name to a hash that can be sent via UMA.
+int DllNameToHash(std::string dll_name);
+
+#endif  // CHROME_ELF_DLL_HASH_DLL_HASH_H_
diff --git a/chrome_elf/dll_hash/dll_hash_main.cc b/chrome_elf/dll_hash/dll_hash_main.cc
new file mode 100644
index 0000000..a360263
--- /dev/null
+++ b/chrome_elf/dll_hash/dll_hash_main.cc
@@ -0,0 +1,28 @@
+// Copyright 2014 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.
+//
+// This is a utility executable used for generating hashes for dll names
+// for inclusion in tools/metrics/histograms/histograms.xml. Every
+// dll name must have a corresponding entry in the enum there.
+
+#include <stdio.h>
+#include <stdlib.h>
+#include <string.h>
+
+#include "chrome_elf/dll_hash/dll_hash.h"
+
+int main(int argc, char** argv) {
+  if (argc < 2) {
+    fprintf(stderr, "Usage: %s <dll name> <dll name> <...>\n", argv[0]);
+    fprintf(stderr, "\n");
+    fprintf(stderr, "Prints hashes for dll names.\n");
+    fprintf(stderr, "Example: %s \"my_dll.dll\" \"user32.dll\"\n", argv[0]);
+    return 1;
+  }
+  for (int i = 1; i < argc; i++) {
+    int hash = DllNameToHash(std::string(argv[i]));
+    printf("<int value=\"%d\" label=\"%s\"/>\n", hash, argv[i]);
+  }
+  return 0;
+}