From da69b9063e8c913434ac3d469cb132dbb465dee5 Mon Sep 17 00:00:00 2001 From: Alex Gough Date: Mon, 13 Feb 2023 20:55:53 +0000 Subject: [PATCH] Move some //base winternl functions to ntdll.lib "Documented" functions in winternl.h can be linked to ntdll.lib rather than getprocaddressed. This does this for those functions in //base where this makes sense. Bug: 1282011 Change-Id: I1edad2db3f9194bbca1a4143930a0e8ed750468e Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4230192 Reviewed-by: Tushar Agarwal Reviewed-by: Will Harris Commit-Queue: Alex Gough Cr-Commit-Position: refs/heads/main@{#1104656} --- base/BUILD.gn | 3 +++ base/process/process_metrics_win.cc | 12 +++------ .../suspendable_thread_delegate_win.cc | 25 ++++--------------- base/win/scoped_handle_unittest.cc | 6 +---- base/win/security_util.cc | 9 ++----- 5 files changed, 14 insertions(+), 41 deletions(-) diff --git a/base/BUILD.gn b/base/BUILD.gn index 9abf3947622d13..e3fc48cfbed717 100644 --- a/base/BUILD.gn +++ b/base/BUILD.gn @@ -1913,6 +1913,7 @@ component("base") { libs += [ "cfgmgr32.lib", + "ntdll.lib", "powrprof.lib", "propsys.lib", "setupapi.lib", @@ -3458,6 +3459,8 @@ test("base_unittests") { if (enable_cet_shadow_stack) { sources += [ "win/cet_shadow_stack_unittest.cc" ] } + + libs = [ "ntdll.lib" ] } if (is_linux || is_chromeos) { diff --git a/base/process/process_metrics_win.cc b/base/process/process_metrics_win.cc index af11f92b51b489..c8b79fe47f3749 100644 --- a/base/process/process_metrics_win.cc +++ b/base/process/process_metrics_win.cc @@ -286,20 +286,14 @@ Value::Dict SystemPerformanceInfo::ToDict() const { // Retrieves performance counters from the operating system. // Fills in the provided |info| structure. Returns true on success. BASE_EXPORT bool GetSystemPerformanceInfo(SystemPerformanceInfo* info) { - static const auto query_system_information_ptr = - reinterpret_cast(GetProcAddress( - GetModuleHandle(L"ntdll.dll"), "NtQuerySystemInformation")); - if (!query_system_information_ptr) - return false; - SYSTEM_PERFORMANCE_INFORMATION counters = {}; { // The call to NtQuerySystemInformation might block on a lock. base::ScopedBlockingCall scoped_blocking_call(FROM_HERE, BlockingType::MAY_BLOCK); - if (query_system_information_ptr(::SystemPerformanceInformation, &counters, - sizeof(SYSTEM_PERFORMANCE_INFORMATION), - nullptr) != STATUS_SUCCESS) { + if (::NtQuerySystemInformation(::SystemPerformanceInformation, &counters, + sizeof(SYSTEM_PERFORMANCE_INFORMATION), + nullptr) != STATUS_SUCCESS) { return false; } } diff --git a/base/profiler/suspendable_thread_delegate_win.cc b/base/profiler/suspendable_thread_delegate_win.cc index 1b9b5010374603..24191d34612772 100644 --- a/base/profiler/suspendable_thread_delegate_win.cc +++ b/base/profiler/suspendable_thread_delegate_win.cc @@ -11,7 +11,7 @@ #include "base/check.h" #include "base/debug/alias.h" -#include "base/memory/raw_ptr.h" +#include "base/memory/raw_ptr_exclusion.h" #include "base/profiler/native_unwinder_win.h" #include "build/build_config.h" @@ -75,34 +75,19 @@ const TEB* GetThreadEnvironmentBlock(PlatformThreadId thread_id, if (thread_id == ::GetCurrentThreadId()) return reinterpret_cast(NtCurrentTeb()); - // Define the internal types we need to invoke NtQueryInformationThread. - enum THREAD_INFORMATION_CLASS { ThreadBasicInformation }; - - struct CLIENT_ID { - HANDLE UniqueProcess; - HANDLE UniqueThread; - }; - + // Define types not in winternl.h needed to invoke NtQueryInformationThread(). + constexpr auto ThreadBasicInformation = static_cast(0); struct THREAD_BASIC_INFORMATION { NTSTATUS ExitStatus; - raw_ptr Teb; + RAW_PTR_EXCLUSION TEB* Teb; // Filled in by the OS so cannot use raw_ptr<>. CLIENT_ID ClientId; KAFFINITY AffinityMask; LONG Priority; LONG BasePriority; }; - using NtQueryInformationThreadFunction = - NTSTATUS(WINAPI*)(HANDLE, THREAD_INFORMATION_CLASS, PVOID, ULONG, PULONG); - - static const auto nt_query_information_thread = - reinterpret_cast(::GetProcAddress( - ::GetModuleHandle(L"ntdll.dll"), "NtQueryInformationThread")); - if (!nt_query_information_thread) - return nullptr; - THREAD_BASIC_INFORMATION basic_info = {0}; - NTSTATUS status = nt_query_information_thread( + NTSTATUS status = ::NtQueryInformationThread( thread_handle, ThreadBasicInformation, &basic_info, sizeof(THREAD_BASIC_INFORMATION), nullptr); if (status != 0) diff --git a/base/win/scoped_handle_unittest.cc b/base/win/scoped_handle_unittest.cc index d6f74e06e31e15..42aefebfa4abbb 100644 --- a/base/win/scoped_handle_unittest.cc +++ b/base/win/scoped_handle_unittest.cc @@ -72,15 +72,11 @@ TEST_F(ScopedHandleTest, ScopedHandle) { TEST_F(ScopedHandleDeathTest, HandleVerifierTrackedHasBeenClosed) { HANDLE handle = ::CreateMutex(nullptr, false, nullptr); ASSERT_NE(HANDLE(nullptr), handle); - using NtCloseFunc = decltype(&::NtClose); - NtCloseFunc ntclose = reinterpret_cast( - GetProcAddress(GetModuleHandle(L"ntdll.dll"), "NtClose")); - ASSERT_NE(nullptr, ntclose); ASSERT_DEATH( { base::win::ScopedHandle handle_holder(handle); - ntclose(handle); + ::NtClose(handle); // Destructing a ScopedHandle with an illegally closed handle should // fail. }, diff --git a/base/win/security_util.cc b/base/win/security_util.cc index e1c1c8f385472b..49d4693fbd3d66 100644 --- a/base/win/security_util.cc +++ b/base/win/security_util.cc @@ -99,14 +99,9 @@ void AppendSidVector(std::vector& base_sids, } absl::optional GetGrantedAccess(HANDLE handle) { - static const auto query_object = reinterpret_cast( - GetProcAddress(::GetModuleHandle(L"ntdll.dll"), "NtQueryObject")); - if (!query_object) { - return absl::nullopt; - } PUBLIC_OBJECT_BASIC_INFORMATION basic_info = {}; - if (!NT_SUCCESS(query_object(handle, ObjectBasicInformation, &basic_info, - sizeof(basic_info), nullptr))) { + if (!NT_SUCCESS(::NtQueryObject(handle, ObjectBasicInformation, &basic_info, + sizeof(basic_info), nullptr))) { return absl::nullopt; } return basic_info.GrantedAccess;