From: Iain Patterson Date: Tue, 1 Apr 2014 21:20:21 +0000 (+0100) Subject: Fixed broken environment on restart. X-Git-Tag: v2.22~3 X-Git-Url: http://git.iain.cx/?a=commitdiff_plain;h=620fc9f56975c9319a1b0e0be556e264ad1d485d;p=nssm.git Fixed broken environment on restart. We were calling duplicate_environment() to copy the service's initial environment prior to restarting it. However the duplicate_environment() function modified its input by replacing = characters with NULLs. That is not allowed; the memory returned by GetEnvironmentStrings() must be treated as readonly. After two restarts the application would run with a null environment and probably crash. We now copy the initial environment on to the heap and operate on the copy, ensuring that the corruption does not occur. Thanks Alessandro Gherardi. --- diff --git a/README.txt b/README.txt index 32051ab..b0d32a7 100644 --- a/README.txt +++ b/README.txt @@ -679,6 +679,8 @@ Thanks to Andrew RedzMax for suggesting an unconditional restart delay. Thanks to Bryan Senseman for noticing that applications with redirected stdout and/or stderr which attempt to read from stdin would fail. Thanks to Czenda Czendov for help with Visual Studio 2013 and Server 2012R2. +Thanks to Alessandro Gherardi for reporting and draft fix of the bug whereby +the second restart of the application would have a corrupted environment. Licence ------- diff --git a/env.cpp b/env.cpp index 76c187e..063b60f 100644 --- a/env.cpp +++ b/env.cpp @@ -1,5 +1,23 @@ #include "nssm.h" +/* Copy an environment block. */ +TCHAR *copy_environment_block(TCHAR *env) { + unsigned long len; + + if (! env) return 0; + for (len = 0; env[len]; len++) while (env[len]) len++; + if (! len++) return 0; + + TCHAR *newenv = (TCHAR *) HeapAlloc(GetProcessHeap(), 0, len * sizeof(TCHAR)); + if (! newenv) { + log_event(EVENTLOG_ERROR_TYPE, NSSM_EVENT_OUT_OF_MEMORY, _T("environment"), _T("copy_environment_block()"), 0); + return 0; + } + + memmove(newenv, env, len * sizeof(TCHAR)); + return newenv; +} + /* The environment block starts with variables of the form =C:=C:\Windows\System32 which we ignore. @@ -139,3 +157,17 @@ int test_environment(TCHAR *env) { return 0; } + +/* + Duplicate an environment block returned by GetEnvironmentStrings(). + Since such a block is by definition readonly, and duplicate_environment() + modifies its inputs, this function takes a copy of the input and operates + on that. +*/ +void duplicate_environment_strings(TCHAR *env) { + TCHAR *newenv = copy_environment_block(env); + if (! newenv) return; + + duplicate_environment(newenv); + HeapFree(GetProcessHeap(), 0, newenv); +} diff --git a/env.h b/env.h index 24d8987..021ff4f 100644 --- a/env.h +++ b/env.h @@ -1,11 +1,13 @@ #ifndef ENV_H #define ENV_H +TCHAR *copy_environment_block(TCHAR *); TCHAR *useful_environment(TCHAR *); TCHAR *expand_environment_string(TCHAR *); int set_environment_block(TCHAR *); int clear_environment(); int duplicate_environment(TCHAR *); int test_environment(TCHAR *); +void duplicate_environment_strings(TCHAR *); #endif diff --git a/service.cpp b/service.cpp index 5b10233..8d1c0fe 100644 --- a/service.cpp +++ b/service.cpp @@ -1623,7 +1623,7 @@ int start_service(nssm_service_t *service) { unsigned long error = GetLastError(); log_event(EVENTLOG_ERROR_TYPE, NSSM_EVENT_CREATEPROCESS_FAILED, service->name, service->exe, error_string(error), 0); close_output_handles(&si); - duplicate_environment(service->initial_env); + duplicate_environment_strings(service->initial_env); return stop_service(service, exitcode, true, true); } service->process_handle = pi.hProcess; @@ -1636,7 +1636,7 @@ int start_service(nssm_service_t *service) { if (! service->no_console) FreeConsole(); /* Restore our environment. */ - duplicate_environment(service->initial_env); + duplicate_environment_strings(service->initial_env); if (service->affinity) { /*