From 5f4067db3b7c722a3e0176fd3eee4f94ea9c3a13 Mon Sep 17 00:00:00 2001 From: Mihai Sarbulescu Date: Fri, 24 Jun 2022 17:15:33 +0300 Subject: [PATCH 1/2] Issue 903261 timeout not working on shell command when using elevation --- source/code/include/scxcorelib/scxprocess.h | 1 + source/code/scxcorelib/pal/scxprocess.cpp | 45 ++++++++++++++++++++- source/code/scxcorelib/pal/scxthread.cpp | 30 ++++++++++---- 3 files changed, 68 insertions(+), 8 deletions(-) diff --git a/source/code/include/scxcorelib/scxprocess.h b/source/code/include/scxcorelib/scxprocess.h index fae52d8..80ed637 100644 --- a/source/code/include/scxcorelib/scxprocess.h +++ b/source/code/include/scxcorelib/scxprocess.h @@ -108,6 +108,7 @@ namespace SCXCoreLib virtual ~SCXProcess(); void Kill(); unsigned GetEffectiveTimeout(unsigned timeout); + std::wstring ConstructShellCommandWithElevation(const std::wstring &command, const std::wstring &elevationtype); protected: virtual bool ReadToStream(int fd, std::ostream& stream); diff --git a/source/code/scxcorelib/pal/scxprocess.cpp b/source/code/scxcorelib/pal/scxprocess.cpp index 4c64943..377319d 100644 --- a/source/code/scxcorelib/pal/scxprocess.cpp +++ b/source/code/scxcorelib/pal/scxprocess.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include @@ -798,10 +799,52 @@ namespace SCXCoreLib { if (killpg(m_pid, SIGKILL) < 0 && errno != ESRCH) { - throw SCXInternalErrorException(UnexpectedErrno(L"Unable to kill child process group", errno), SCXSRCLOCATION); + // EPERM because the child omiagent process is not running as root (sudo) + // and thus will not have priviledges to send signals to this root (sudo) process + if (errno == EPERM) + { + std::istringstream processInput; + std::ostringstream processOutput; + std::ostringstream processError; + + wchar_t procId[10]; + swprintf(procId, 10, L"%d", m_pid); + + std::wstring command(L"kill -9 "); + command += procId; + std::wstring elevatedCommand = ConstructShellCommandWithElevation(command, L"sudo"); + + // best effort to try to kill this using "kill" command with SUDO elevation + // we might want to do some error handling here as well ... + SCXCoreLib::SCXProcess::Run(SplitCommand(elevatedCommand), processInput, processOutput, processError, 5000); + } + else + { + throw SCXInternalErrorException(UnexpectedErrno(L"Unable to kill child process group", errno), SCXSRCLOCATION); + } } } + /**********************************************************************************/ + //! Construct shell command with elevation + std::wstring SCXProcess::ConstructShellCommandWithElevation(const std::wstring &command, + const std::wstring &elevationtype) + { + SCXSystemLib::SystemInfo si; + + std::wstring newCommand(si.GetShellCommand(command)); + + // Only when current user is not priviledged and elevation type is sudo + // the command need to be elevated. + // Force a shell command so we get a shell (even if already elevated) + if (elevationtype == L"sudo") + { + newCommand = si.GetElevatedCommand(newCommand); + } + + return newCommand; + } + /**********************************************************************************/ //! Terminate the parent process, explicitly closing STDIN/OUT/ERR (so they flush) void SCXProcess::CloseAndDie() diff --git a/source/code/scxcorelib/pal/scxthread.cpp b/source/code/scxcorelib/pal/scxthread.cpp index 3cc20ef..9473824 100644 --- a/source/code/scxcorelib/pal/scxthread.cpp +++ b/source/code/scxcorelib/pal/scxthread.cpp @@ -16,6 +16,7 @@ #include #include #include +#include #include #if defined(WIN32) #elif defined(SCX_UNIX) @@ -62,12 +63,12 @@ namespace SCXCoreLib void* _InternalThreadStartRoutine(void* param) #endif { + int retCode = 0; ThreadStartParams* p = static_cast(param); SCXASSERT(p != 0); if (p->body != 0) { -#if !defined(_DEBUG) try { p->body( p->param ); @@ -76,20 +77,35 @@ namespace SCXCoreLib { SCXASSERTFAIL(std::wstring(L"ThreadStartRoutine() Thread threw unhandled exception - "). append(e1.What()).append(L" - ").append(e1.Where()).c_str()); + + // We don't want to have the log variable as a member variable, + // because we don't want to open a new handle on every new thread, but rather only on exception + SCXCoreLib::SCXLogHandle log = SCXLogHandleFactory::GetLogHandle(std::wstring(L"scx.core.common.pal.process")); + SCX_LOGERROR(log, e1.What()); + + retCode = -1; } catch (const std::exception& e2) { SCXASSERTFAIL(std::wstring(L"ThreadStartRoutine() Thread threw unhandled exception - "). append(StrFromUTF8(e2.what())).c_str()); + + // We don't want to have the log variable as a member variable, + // because we don't want to open a new handle on every new thread, but rather only on exception + SCXCoreLib::SCXLogHandle log = SCXLogHandleFactory::GetLogHandle(std::wstring(L"scx.core.common.pal.process")); + SCX_LOGERROR(log, e2.what()); + + retCode = -1; } -#else - p->body( p->param ); -#endif - /* We would like to catch (...) as well but it seemes we can't since there is a bug - in gcc. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=28145 */ } + delete p; - return 0; + +#if defined(WIN32) + return reinterpret_cast(retCode); +#elif defined(SCX_UNIX) + return reinterpret_cast(retCode); +#endif } } From 94f32794f09dcbe7a6dd86d7a298c7d2ab4b6e44 Mon Sep 17 00:00:00 2001 From: Mihai Sarbulescu Date: Fri, 24 Jun 2022 17:23:36 +0300 Subject: [PATCH 2/2] modified comment --- source/code/scxcorelib/pal/scxprocess.cpp | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/source/code/scxcorelib/pal/scxprocess.cpp b/source/code/scxcorelib/pal/scxprocess.cpp index 377319d..53c4682 100644 --- a/source/code/scxcorelib/pal/scxprocess.cpp +++ b/source/code/scxcorelib/pal/scxprocess.cpp @@ -815,7 +815,8 @@ namespace SCXCoreLib std::wstring elevatedCommand = ConstructShellCommandWithElevation(command, L"sudo"); // best effort to try to kill this using "kill" command with SUDO elevation - // we might want to do some error handling here as well ... + // we don't need any error handling here because this will throw an exception if something happens + // and that will get logged in scx log and this it a best effort in trying to kill the process SCXCoreLib::SCXProcess::Run(SplitCommand(elevatedCommand), processInput, processOutput, processError, 5000); } else