From 920d4e96f5b6699890b7f3bd8dc1ba37ffb3cf48 Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Sat, 14 Jan 2023 18:50:11 +0200 Subject: [PATCH 1/3] Fix child process spawning on linux --- src/platform/common.h | 3 ++- src/platform/linux/misc.cpp | 6 +++--- src/platform/macos/misc.cpp | 6 +++--- src/platform/windows/misc.cpp | 4 ++-- src/process.cpp | 8 ++++---- 5 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/platform/common.h b/src/platform/common.h index 468df86323f..7d1c13f3c4b 100644 --- a/src/platform/common.h +++ b/src/platform/common.h @@ -28,6 +28,7 @@ class path; } namespace process { class child; +class group; template class basic_environment; typedef basic_environment environment; @@ -311,7 +312,7 @@ std::shared_ptr display(mem_type_e hwdevice_type, const std::string & // A list of names of displays accepted as display_name with the mem_type_e std::vector display_names(mem_type_e hwdevice_type); -boost::process::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, boost::process::environment &env, FILE *file, std::error_code &ec); +boost::process::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, boost::process::environment &env, FILE *file, std::error_code &ec, boost::process::group &group); enum class thread_priority_e : int { low, diff --git a/src/platform/linux/misc.cpp b/src/platform/linux/misc.cpp index 35e68b96c3a..879a9db9cf3 100644 --- a/src/platform/linux/misc.cpp +++ b/src/platform/linux/misc.cpp @@ -143,13 +143,13 @@ std::string get_mac_address(const std::string_view &address) { return "00:00:00:00:00:00"s; } -bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec) { +bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group &group) { BOOST_LOG(warning) << "run_unprivileged() is not yet implemented for this platform. The new process will run with Sunshine's permissions."sv; if(!file) { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec); + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec, group); } else { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec); + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec, group); } } diff --git a/src/platform/macos/misc.cpp b/src/platform/macos/misc.cpp index eb99de78d2b..44f2a0e381a 100644 --- a/src/platform/macos/misc.cpp +++ b/src/platform/macos/misc.cpp @@ -121,13 +121,13 @@ std::string get_mac_address(const std::string_view &address) { return "00:00:00:00:00:00"s; } -bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec) { +bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group &group) { BOOST_LOG(warning) << "run_unprivileged() is not yet implemented for this platform. The new process will run with Sunshine's permissions."sv; if(!file) { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec); + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec, group); } else { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec); + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec, group); } } diff --git a/src/platform/windows/misc.cpp b/src/platform/windows/misc.cpp index 9b6867daf23..5793e63a058 100644 --- a/src/platform/windows/misc.cpp +++ b/src/platform/windows/misc.cpp @@ -345,7 +345,7 @@ void free_proc_thread_attr_list(LPPROC_THREAD_ATTRIBUTE_LIST list) { HeapFree(GetProcessHeap(), 0, list); } -bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec) { +bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group &group) { HANDLE shell_token = duplicate_shell_token(); if(!shell_token) { // This can happen if the shell has crashed. Fail the launch rather than risking launching with @@ -459,7 +459,7 @@ bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &work if(ret) { // Since we are always spawning a process with a less privileged token than ourselves, // bp::child() should have no problem opening it with any access rights it wants. - auto child = bp::child((bp::pid_t)process_info.dwProcessId); + auto child = bp::child((bp::pid_t)process_info.dwProcessId, group); // Only close handles after bp::child() has opened the process. If the process terminates // quickly, the PID could be reused if we close the process handle. diff --git a/src/process.cpp b/src/process.cpp index 21e633aee7e..65162f89fb2 100644 --- a/src/process.cpp +++ b/src/process.cpp @@ -150,12 +150,14 @@ int proc_t::execute(int app_id) { find_working_directory(cmd, _env) : boost::filesystem::path(proc.working_dir); BOOST_LOG(info) << "Spawning ["sv << cmd << "] in ["sv << working_dir << ']'; - auto child = platf::run_unprivileged(cmd, working_dir, _env, _pipe.get(), ec); + bp::group child_group; + auto child = platf::run_unprivileged(cmd, working_dir, _env, _pipe.get(), ec, child_group); if(ec) { BOOST_LOG(warning) << "Couldn't spawn ["sv << cmd << "]: System: "sv << ec.message(); } else { child.detach(); + child_group.detach(); } } @@ -168,13 +170,11 @@ int proc_t::execute(int app_id) { find_working_directory(proc.cmd, _env) : boost::filesystem::path(proc.working_dir); BOOST_LOG(info) << "Executing: ["sv << proc.cmd << "] in ["sv << working_dir << ']'; - _process = platf::run_unprivileged(proc.cmd, working_dir, _env, _pipe.get(), ec); + _process = platf::run_unprivileged(proc.cmd, working_dir, _env, _pipe.get(), ec, _process_handle); if(ec) { BOOST_LOG(warning) << "Couldn't run ["sv << proc.cmd << "]: System: "sv << ec.message(); return -1; } - - _process_handle.add(_process); } fg.disable(); From 230f222a2a853b0aa99e20decf4d5cfc11ec0b0e Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Sat, 14 Jan 2023 22:22:35 +0200 Subject: [PATCH 2/3] Drop the temporary child group --- src/platform/common.h | 2 +- src/platform/linux/misc.cpp | 18 ++++++++++++++---- src/platform/macos/misc.cpp | 18 ++++++++++++++---- src/platform/windows/misc.cpp | 4 ++-- src/process.cpp | 6 ++---- 5 files changed, 33 insertions(+), 15 deletions(-) diff --git a/src/platform/common.h b/src/platform/common.h index 7d1c13f3c4b..e5b18b6088b 100644 --- a/src/platform/common.h +++ b/src/platform/common.h @@ -312,7 +312,7 @@ std::shared_ptr display(mem_type_e hwdevice_type, const std::string & // A list of names of displays accepted as display_name with the mem_type_e std::vector display_names(mem_type_e hwdevice_type); -boost::process::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, boost::process::environment &env, FILE *file, std::error_code &ec, boost::process::group &group); +boost::process::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, boost::process::environment &env, FILE *file, std::error_code &ec, boost::process::group *group); enum class thread_priority_e : int { low, diff --git a/src/platform/linux/misc.cpp b/src/platform/linux/misc.cpp index 879a9db9cf3..1e24965f0e0 100644 --- a/src/platform/linux/misc.cpp +++ b/src/platform/linux/misc.cpp @@ -143,13 +143,23 @@ std::string get_mac_address(const std::string_view &address) { return "00:00:00:00:00:00"s; } -bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group &group) { +bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) { BOOST_LOG(warning) << "run_unprivileged() is not yet implemented for this platform. The new process will run with Sunshine's permissions."sv; - if(!file) { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec, group); + if(!group) { + if(!file) { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec); + } + else { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec); + } } else { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec, group); + if(!file) { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec, *group); + } + else { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec, *group); + } } } diff --git a/src/platform/macos/misc.cpp b/src/platform/macos/misc.cpp index 44f2a0e381a..880b9f30c8b 100644 --- a/src/platform/macos/misc.cpp +++ b/src/platform/macos/misc.cpp @@ -121,13 +121,23 @@ std::string get_mac_address(const std::string_view &address) { return "00:00:00:00:00:00"s; } -bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group &group) { +bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) { BOOST_LOG(warning) << "run_unprivileged() is not yet implemented for this platform. The new process will run with Sunshine's permissions."sv; - if(!file) { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec, group); + if(!group) { + if(!file) { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec); + } + else { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec); + } } else { - return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec, group); + if(!file) { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > bp::null, bp::std_err > bp::null, ec, *group); + } + else { + return bp::child(cmd, env, bp::start_dir(working_dir), bp::std_out > file, bp::std_err > file, ec, *group); + } } } diff --git a/src/platform/windows/misc.cpp b/src/platform/windows/misc.cpp index 5793e63a058..6ccd0e74e09 100644 --- a/src/platform/windows/misc.cpp +++ b/src/platform/windows/misc.cpp @@ -345,7 +345,7 @@ void free_proc_thread_attr_list(LPPROC_THREAD_ATTRIBUTE_LIST list) { HeapFree(GetProcessHeap(), 0, list); } -bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group &group) { +bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &working_dir, bp::environment &env, FILE *file, std::error_code &ec, bp::group *group) { HANDLE shell_token = duplicate_shell_token(); if(!shell_token) { // This can happen if the shell has crashed. Fail the launch rather than risking launching with @@ -459,7 +459,7 @@ bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &work if(ret) { // Since we are always spawning a process with a less privileged token than ourselves, // bp::child() should have no problem opening it with any access rights it wants. - auto child = bp::child((bp::pid_t)process_info.dwProcessId, group); + auto child = group ? bp::child((bp::pid_t)process_info.dwProcessId, *group) : bp::child((bp::pid_t)process_info.dwProcessId); // Only close handles after bp::child() has opened the process. If the process terminates // quickly, the PID could be reused if we close the process handle. diff --git a/src/process.cpp b/src/process.cpp index 65162f89fb2..3d3bd3e7988 100644 --- a/src/process.cpp +++ b/src/process.cpp @@ -150,14 +150,12 @@ int proc_t::execute(int app_id) { find_working_directory(cmd, _env) : boost::filesystem::path(proc.working_dir); BOOST_LOG(info) << "Spawning ["sv << cmd << "] in ["sv << working_dir << ']'; - bp::group child_group; - auto child = platf::run_unprivileged(cmd, working_dir, _env, _pipe.get(), ec, child_group); + auto child = platf::run_unprivileged(cmd, working_dir, _env, _pipe.get(), ec, nullptr); if(ec) { BOOST_LOG(warning) << "Couldn't spawn ["sv << cmd << "]: System: "sv << ec.message(); } else { child.detach(); - child_group.detach(); } } @@ -170,7 +168,7 @@ int proc_t::execute(int app_id) { find_working_directory(proc.cmd, _env) : boost::filesystem::path(proc.working_dir); BOOST_LOG(info) << "Executing: ["sv << proc.cmd << "] in ["sv << working_dir << ']'; - _process = platf::run_unprivileged(proc.cmd, working_dir, _env, _pipe.get(), ec, _process_handle); + _process = platf::run_unprivileged(proc.cmd, working_dir, _env, _pipe.get(), ec, &_process_handle); if(ec) { BOOST_LOG(warning) << "Couldn't run ["sv << proc.cmd << "]: System: "sv << ec.message(); return -1; From fd3ba0a2e3b277c807b3f058544b234d0454760f Mon Sep 17 00:00:00 2001 From: FrogTheFrog Date: Wed, 18 Jan 2023 17:48:48 +0200 Subject: [PATCH 3/3] Add child to group via method instead of ctor on Windows --- src/platform/windows/misc.cpp | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/src/platform/windows/misc.cpp b/src/platform/windows/misc.cpp index 1ef2878c23f..c89b0f2c412 100644 --- a/src/platform/windows/misc.cpp +++ b/src/platform/windows/misc.cpp @@ -489,7 +489,10 @@ bp::child run_unprivileged(const std::string &cmd, boost::filesystem::path &work if(ret) { // Since we are always spawning a process with a less privileged token than ourselves, // bp::child() should have no problem opening it with any access rights it wants. - auto child = group ? bp::child((bp::pid_t)process_info.dwProcessId, *group) : bp::child((bp::pid_t)process_info.dwProcessId); + auto child = bp::child((bp::pid_t)process_info.dwProcessId); + if(group) { + group->add(child); + } // Only close handles after bp::child() has opened the process. If the process terminates // quickly, the PID could be reused if we close the process handle.