Fix race condition inserting new process into our job object

Before this fix, it could spawn a child that would escape our tracking if it ran before we added it to our job object.
This commit is contained in:
Cameron Gutman 2023-08-14 19:31:51 -05:00
parent 1086d4dfa2
commit 774f13aecb

View File

@ -40,6 +40,11 @@
#define UDP_SEND_MSG_SIZE 2
#endif
// PROC_THREAD_ATTRIBUTE_JOB_LIST is currently missing from MinGW headers
#ifndef PROC_THREAD_ATTRIBUTE_JOB_LIST
#define PROC_THREAD_ATTRIBUTE_JOB_LIST ProcThreadAttributeValue(13, FALSE, TRUE, FALSE)
#endif
#include <qos2.h>
#ifndef WLAN_API_MAKE_VERSION
@ -411,11 +416,10 @@ namespace platf {
* @param cmd The command that was used to launch the process.
* @param ec A reference to an `std::error_code` object that will store any error that occurred during the launch.
* @param process_info A reference to a `PROCESS_INFORMATION` structure that contains information about the new process.
* @param group A pointer to a `bp::group` object that will add the new process to its group, if not null.
* @return A `bp::child` object representing the new process, or an empty `bp::child` object if the launch failed.
*/
bp::child
create_boost_child_from_results(bool process_launched, const std::string &cmd, std::error_code &ec, PROCESS_INFORMATION &process_info, bp::group *group) {
create_boost_child_from_results(bool process_launched, const std::string &cmd, std::error_code &ec, PROCESS_INFORMATION &process_info) {
// Use RAII to ensure the process is closed when we're done with it, even if there was an error.
auto close_process_handles = util::fail_guard([process_launched, process_info]() {
if (process_launched) {
@ -432,11 +436,6 @@ namespace platf {
if (process_launched) {
// If the launch was successful, create a new bp::child object representing the new process
auto child = bp::child((bp::pid_t) process_info.dwProcessId);
if (group) {
// If a group was provided, add the new process to the group
group->add(child);
}
BOOST_LOG(info) << cmd << " running with PID "sv << child.id();
return child;
}
@ -491,17 +490,18 @@ namespace platf {
/**
* @brief A function to create a `STARTUPINFOEXW` structure for launching a process.
* @param file A pointer to a `FILE` object that will be used as the standard output and error for the new process, or null if not needed.
* @param job A job object handle to insert the new process into. This pointer must remain valid for the life of this startup info!
* @param ec A reference to a `std::error_code` object that will store any error that occurred during the creation of the structure.
* @return A `STARTUPINFOEXW` structure that contains information about how to launch the new process.
*/
STARTUPINFOEXW
create_startup_info(FILE *file, std::error_code &ec) {
create_startup_info(FILE *file, HANDLE *job, std::error_code &ec) {
// Initialize a zeroed-out STARTUPINFOEXW structure and set its size
STARTUPINFOEXW startup_info = {};
startup_info.StartupInfo.cb = sizeof(startup_info);
// Allocate a process attribute list with space for 1 element
startup_info.lpAttributeList = allocate_proc_thread_attr_list(1);
// Allocate a process attribute list with space for 2 elements
startup_info.lpAttributeList = allocate_proc_thread_attr_list(2);
if (startup_info.lpAttributeList == NULL) {
// If the allocation failed, set ec to an appropriate error code and return the structure
ec = std::make_error_code(std::errc::not_enough_memory);
@ -532,6 +532,20 @@ namespace platf {
NULL);
}
if (job) {
// Atomically insert the new process into the specified job.
//
// Note: The value we point to here must be valid for the lifetime of the attribute list,
// so we take a HANDLE* instead of just a HANDLE to use the caller's stack storage.
UpdateProcThreadAttribute(startup_info.lpAttributeList,
0,
PROC_THREAD_ATTRIBUTE_JOB_LIST,
job,
sizeof(*job),
NULL,
NULL);
}
return startup_info;
}
@ -557,7 +571,8 @@ namespace platf {
std::wstring wcmd = converter.from_bytes(cmd);
std::wstring start_dir = converter.from_bytes(working_dir.string());
STARTUPINFOEXW startup_info = create_startup_info(file, ec);
HANDLE job = group ? group->native_handle() : nullptr;
STARTUPINFOEXW startup_info = create_startup_info(file, job ? &job : nullptr, ec);
PROCESS_INFORMATION process_info;
// Clone the environment to create a local copy. Boost.Process (bp) shares the environment with all spawned processes.
@ -649,7 +664,7 @@ namespace platf {
}
// Use the results of the launch to create a bp::child object
return create_boost_child_from_results(ret, cmd, ec, process_info, group);
return create_boost_child_from_results(ret, cmd, ec, process_info);
}
/**