https://github.com/Teemperor created https://github.com/llvm/llvm-project/pull/204109
Our current FindProcessImpl has a TOCTOU bug where we first query the buffer size we should provide via sysctl and then later pass a buffer with that size to be filled. If the list of processes grows larger than the buffer we pass, then our current implementation fails by returning an empty list of processes. This race only happens rarely in practice as we pad the buffer size with 10 additional entries to account for some process growth. This patch replaces this logic by a backoff loop that retries fetching the process list if our buffer is too small (sysctl tells us if this is the case by setting ENOMEM). This new implementation can only fail in the last iteration if the system spawns about 130k processes between the two sysctl calls which should be practically impossible. This should fix the actual root-cause for the random failures in TestSimulator.py >From ab116cdf959d0eb4e81df59a06c14239a53aa7b6 Mon Sep 17 00:00:00 2001 From: Raphael Isemann <[email protected]> Date: Tue, 16 Jun 2026 10:40:50 +0100 Subject: [PATCH] [lldb] Fix race in macOS's FindProcessesImpl Our current FindProcessImpl has a TOCTOU bug where we first query the buffer size we should provide via sysctl and then later pass a buffer with that size to be filled. If the list of processes grows larger than the buffer we pass, then our current implementation fails by returning an empty list of processes. This race only happens rarely in practice as we pad the buffer size with 10 additional entries to account for some process growth. This patch replaces this logic by a backoff loop that retries fetching the process list if our buffer is too small (sysctl tells us if this is the case by setting ENOMEM). This new implementation can only fail in the last iteration if the system spawns about 130k processes between the two sysctl calls which should be practically impossible. This should fix the actual root-cause for the random failures in TestSimulator.py --- lldb/source/Host/macosx/objcxx/Host.mm | 62 ++++++++++++++++++-------- 1 file changed, 44 insertions(+), 18 deletions(-) diff --git a/lldb/source/Host/macosx/objcxx/Host.mm b/lldb/source/Host/macosx/objcxx/Host.mm index febbbfb0a384c..22702db8f51fe 100644 --- a/lldb/source/Host/macosx/objcxx/Host.mm +++ b/lldb/source/Host/macosx/objcxx/Host.mm @@ -39,6 +39,7 @@ #include "llvm/TargetParser/Host.h" #include <asl.h> +#include <cerrno> #include <crt_externs.h> #include <cstdio> #include <cstdlib> @@ -684,34 +685,59 @@ static bool GetMacOSXProcessUserAndGroup(ProcessInstanceInfo &process_info) { return false; } -uint32_t Host::FindProcessesImpl(const ProcessInstanceInfoMatch &match_info, - ProcessInstanceInfoList &process_infos) { - std::vector<struct kinfo_proc> kinfos; - +/// Fetches the list of all processes. Returns true on success. +[[nodiscard]] static bool +GetProcessList(std::vector<struct kinfo_proc> &kinfos) { int mib[3] = {CTL_KERN, KERN_PROC, KERN_PROC_ALL}; - size_t pid_data_size = 0; - if (::sysctl(mib, 3, nullptr, &pid_data_size, nullptr, 0) != 0) - return 0; + // This is an inherently racy API. We have to first query the size for our + // buffer and then pass back to sysctl. If more processes spawn between the + // size query and the actual call to fetch, then sysctl returns ENOMEM. + // We keep retrying until we get a passing result. + for (int attempt = 1; attempt < 1024; ++attempt) { + // Fetch the buffer size sysctl would return. + size_t current_pid_size = 0; + if (::sysctl(mib, 3, nullptr, ¤t_pid_size, nullptr, 0) != 0) + return false; - // Add a few extra in case a few more show up - const size_t estimated_pid_count = - (pid_data_size / sizeof(struct kinfo_proc)) + 10; + // We expect some process growth. Start small and keep increasing the + // expected growth each attempt until we can get the sysctl call to + // succeed. + const size_t expected_growth = attempt * 128 * sizeof(struct kinfo_proc); + const size_t estimated_pid_size = current_pid_size + expected_growth; - kinfos.resize(estimated_pid_count); - pid_data_size = kinfos.size() * sizeof(struct kinfo_proc); + // Allocate the buffer sysctl will copy the process list into. + kinfos.resize(estimated_pid_size / sizeof(struct kinfo_proc)); - if (::sysctl(mib, 3, &kinfos[0], &pid_data_size, nullptr, 0) != 0) - return 0; + // Fetch the actual process list and let syctl adjust actual_pid_size. + size_t actual_pid_size = kinfos.size() * sizeof(struct kinfo_proc); + if (::sysctl(mib, 3, &kinfos[0], &actual_pid_size, nullptr, 0) == 0) { + // Adjust the buffer to the actual number of processes returned. + kinfos.resize(actual_pid_size / sizeof(struct kinfo_proc)); + return true; + } - const size_t actual_pid_count = (pid_data_size / sizeof(struct kinfo_proc)); + // Errno is set to ENOMEM if our estimated_pid_size is too small. + if (errno != ENOMEM) + return false; + } + + return false; +} + +uint32_t Host::FindProcessesImpl(const ProcessInstanceInfoMatch &match_info, + ProcessInstanceInfoList &process_infos) { + std::vector<struct kinfo_proc> kinfos; + if (!GetProcessList(kinfos)) { + Log *log(GetLog(LLDBLog::Host | LLDBLog::Process)); + LLDB_LOG(log, "error: Failed to read process list."); + return 0; + } bool all_users = match_info.GetMatchAllUsers(); const lldb::pid_t our_pid = getpid(); const uid_t our_uid = getuid(); - for (size_t i = 0; i < actual_pid_count; i++) { - const struct kinfo_proc &kinfo = kinfos[i]; - + for (const struct kinfo_proc &kinfo : kinfos) { bool kinfo_user_matches = false; if (all_users) kinfo_user_matches = true; _______________________________________________ lldb-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
