Commit 7b362a27 authored by Jesse McKenna's avatar Jesse McKenna Committed by Commit Bot

IdleWakeups: skip processes that report 0 threads

This change fixes IdleWakeups' SystemInformationSampler failing to
handle processes for which ::NtQuerySystemInformation() returns a
thread count of zero. With this change, processes with "zero" threads
are simply skipped.

Currently, the "Secure System" process reports a thread count of 0
(as it prohibits other processes from accessing it in any way), and
SystemInformationSampler subtracts 1 from this (logically assuming
that every process has at least one thread), which overflows to a
large number. SystemInformationSampler, noting that thread
information for <large number> threads would not fit in its buffer,
triggers an early return.

In practice, this results in IdleWakeups showing 0 processes when
a process with "zero" threads exists before its targeted process in
the process list, because it hits the early return described above
before reaching the process of interest.

Change-Id: I63b9a36bb139aba0b18539e65312bdc5d836cc69
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2335799
Commit-Queue: Jesse McKenna <jessemckenna@google.com>
Reviewed-by: default avatarBruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#795542}
parent 1e3da910
......@@ -283,27 +283,31 @@ std::unique_ptr<ProcessDataSnapshot> SystemInformationSampler::TakeSnapshot() {
auto pi = reinterpret_cast<const SYSTEM_PROCESS_INFORMATION*>(
data_buffer.data() + offset);
// Validate that |pi| and any additional SYSTEM_THREAD_INFORMATION structs
// that it may have are all within the buffer boundary.
if (offset + sizeof(SYSTEM_PROCESS_INFORMATION) +
(pi->NumberOfThreads - 1) * sizeof(SYSTEM_THREAD_INFORMATION) >
data_buffer.size()) {
break;
}
if (pi->ImageName.Buffer) {
// Validate that the image name is within the buffer boundary.
// ImageName.Length seems to be in bytes rather than characters.
size_t image_name_offset =
reinterpret_cast<BYTE*>(pi->ImageName.Buffer) - data_buffer.data();
if (image_name_offset + pi->ImageName.Length > data_buffer.size())
// Skip processes that report zero threads (e.g., the "Secure System"
// process, which does not disclose its thread count).
if (pi->NumberOfThreads > 0) {
// Validate that |pi| and any additional SYSTEM_THREAD_INFORMATION structs
// that it may have are all within the buffer boundary.
if (offset + sizeof(SYSTEM_PROCESS_INFORMATION) +
(pi->NumberOfThreads - 1) * sizeof(SYSTEM_THREAD_INFORMATION) >
data_buffer.size()) {
break;
}
// If |pi| is the targeted process, add its data to the snapshot.
if (wcsncmp(target_process_name_filter(), pi->ImageName.Buffer,
lstrlen(target_process_name_filter())) == 0) {
snapshot->processes.insert(
std::make_pair(pi->ProcessId, GetProcessData(pi)));
if (pi->ImageName.Buffer) {
// Validate that the image name is within the buffer boundary.
// ImageName.Length seems to be in bytes rather than characters.
size_t image_name_offset =
reinterpret_cast<BYTE*>(pi->ImageName.Buffer) - data_buffer.data();
if (image_name_offset + pi->ImageName.Length > data_buffer.size())
break;
// If |pi| is the targeted process, add its data to the snapshot.
if (wcsncmp(target_process_name_filter(), pi->ImageName.Buffer,
lstrlen(target_process_name_filter())) == 0) {
snapshot->processes.insert(
std::make_pair(pi->ProcessId, GetProcessData(pi)));
}
}
}
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment