diff options
| -rw-r--r-- | capchild/capture_sync.c | 17 | ||||
| -rw-r--r-- | docbook/release-notes.asciidoc | 6 | ||||
| -rw-r--r-- | sharkd_daemon.c | 8 | ||||
| -rw-r--r-- | wsutil/win32-utils.c | 75 | ||||
| -rw-r--r-- | wsutil/win32-utils.h | 22 | ||||
| -rw-r--r-- | wsutil/ws_pipe.c | 62 | ||||
| -rw-r--r-- | wsutil/ws_pipe.h | 2 |
7 files changed, 108 insertions, 84 deletions
diff --git a/capchild/capture_sync.c b/capchild/capture_sync.c index e0c7e330e8..1dc12b35d0 100644 --- a/capchild/capture_sync.c +++ b/capchild/capture_sync.c @@ -23,6 +23,7 @@ #ifdef _WIN32 #include <wsutil/unicode-utils.h> #include <wsutil/win32-utils.h> +#include <wsutil/ws_pipe.h> #endif #ifdef HAVE_SYS_WAIT_H @@ -219,7 +220,6 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf HANDLE signal_pipe; /* named pipe used to send messages from parent to child (currently only stop) */ GString *args = g_string_sized_new(200); gchar *quoted_arg; - gunichar2 *wcommandline; SECURITY_ATTRIBUTES sa; STARTUPINFO si; PROCESS_INFORMATION pi; @@ -519,11 +519,10 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf g_string_append(args, quoted_arg); g_free(quoted_arg); } - wcommandline = g_utf8_to_utf16(args->str, (glong)args->len, NULL, NULL, NULL); /* call dumpcap */ - if(!CreateProcess(utf_8to16(argv[0]), wcommandline, NULL, NULL, TRUE, - CREATE_NEW_CONSOLE, NULL, NULL, &si, &pi)) { + if(!win32_create_process(argv[0], args->str, NULL, NULL, TRUE, + CREATE_NEW_CONSOLE, NULL, NULL, &si, &pi)) { report_failure("Couldn't run %s in child process: %s", args->str, win32strerror(GetLastError())); ws_close(sync_pipe_read_fd); /* Should close sync_pipe_read */ @@ -531,14 +530,12 @@ sync_pipe_start(capture_options *capture_opts, capture_session *cap_session, inf CloseHandle(signal_pipe); free_argv(argv, argc); g_string_free(args, TRUE); - g_free(wcommandline); return FALSE; } cap_session->fork_child = pi.hProcess; /* We may need to store this and close it later */ CloseHandle(pi.hThread); g_string_free(args, TRUE); - g_free(wcommandline); cap_session->signal_pipe_write_fd = signal_pipe_write_fd; @@ -646,7 +643,6 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, HANDLE data_pipe[2]; /* pipe used to send data from child to parent */ GString *args = g_string_sized_new(200); gchar *quoted_arg; - gunichar2 *wcommandline; SECURITY_ATTRIBUTES sa; STARTUPINFO si; PROCESS_INFORMATION pi; @@ -752,11 +748,10 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, g_string_append(args, quoted_arg); g_free(quoted_arg); } - wcommandline = g_utf8_to_utf16(args->str, (glong)args->len, NULL, NULL, NULL); /* call dumpcap */ - if(!CreateProcess(utf_8to16(argv[0]), wcommandline, NULL, NULL, TRUE, - CREATE_NEW_CONSOLE, NULL, NULL, &si, &pi)) { + if(!win32_create_process(argv[0], args->str, NULL, NULL, TRUE, + CREATE_NEW_CONSOLE, NULL, NULL, &si, &pi)) { *msg = g_strdup_printf("Couldn't run %s in child process: %s", args->str, win32strerror(GetLastError())); ws_close(*data_read_fd); /* Should close data_pipe[PIPE_READ] */ @@ -764,14 +759,12 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, ws_close(*message_read_fd); /* Should close sync_pipe[PIPE_READ] */ CloseHandle(sync_pipe[PIPE_WRITE]); g_string_free(args, TRUE); - g_free(wcommandline); return -1; } *fork_child = pi.hProcess; /* We may need to store this and close it later */ CloseHandle(pi.hThread); g_string_free(args, TRUE); - g_free(wcommandline); #else /* _WIN32 */ /* Create a pipe for the child process to send us messages */ if (pipe(sync_pipe) < 0) { diff --git a/docbook/release-notes.asciidoc b/docbook/release-notes.asciidoc index 0696b793a3..aba8659ac4 100644 --- a/docbook/release-notes.asciidoc +++ b/docbook/release-notes.asciidoc @@ -34,6 +34,9 @@ Features” section below for more details. //_Non-empty section placeholder._ +Dumpcap might not quit if Wireshark or TShark crashes. +(ws_buglink:1419[]) + === New and Updated Features The following features are new (or have been significantly updated) @@ -172,9 +175,6 @@ locations on your system. == Known Problems -Dumpcap might not quit if Wireshark or TShark crashes. -(ws_buglink:1419[]) - The BER dissector might infinitely loop. (ws_buglink:1516[]) diff --git a/sharkd_daemon.c b/sharkd_daemon.c index 4f98ef48cb..89af1ebcb5 100644 --- a/sharkd_daemon.c +++ b/sharkd_daemon.c @@ -37,6 +37,7 @@ #endif #include <wsutil/strtoi.h> +#include <wsutil/win32-utils.h> #include "sharkd.h" @@ -232,7 +233,6 @@ sharkd_loop(void) PROCESS_INFORMATION pi; STARTUPINFO si; char *exename; - gunichar2 *commandline; #endif socket_handle_t fd; @@ -273,11 +273,10 @@ sharkd_loop(void) si.hStdError = GetStdHandle(STD_ERROR_HANDLE); exename = g_strdup_printf("%s\\%s", get_progfile_dir(), "sharkd.exe"); - commandline = g_utf8_to_utf16("sharkd.exe -", -1, NULL, NULL, NULL); - if (!CreateProcess(utf_8to16(exename), commandline, NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) + if (!win32_create_process(exename, "sharkd.exe -", NULL, NULL, TRUE, 0, NULL, NULL, &si, &pi)) { - fprintf(stderr, "CreateProcess(%s) failed\n", exename); + fprintf(stderr, "win32_create_process(%s) failed\n", exename); } else { @@ -285,7 +284,6 @@ sharkd_loop(void) } g_free(exename); - g_free(commandline); #endif closesocket(fd); diff --git a/wsutil/win32-utils.c b/wsutil/win32-utils.c index 9de0723652..b0541a2021 100644 --- a/wsutil/win32-utils.c +++ b/wsutil/win32-utils.c @@ -8,8 +8,14 @@ * SPDX-License-Identifier: GPL-2.0-or-later */ +#include <config.h> + #include "win32-utils.h" +#include <log.h> + +#include <tchar.h> + /* Quote the argument element if necessary, so that it will get * reconstructed correctly in the C runtime startup code. Note that * the unquoting algorithm in the C runtime is really weird, and @@ -149,15 +155,78 @@ win32strexception(DWORD exception) return errbuf; } +// This appears to be the closest equivalent to SIGPIPE on Windows. +// https://blogs.msdn.microsoft.com/oldnewthing/20131209-00/?p=2433 +// https://stackoverflow.com/a/53214/82195 + +static void win32_kill_child_on_exit(HANDLE child_handle) { + static HANDLE cjo_handle = NULL; + if (!cjo_handle) { + cjo_handle = CreateJobObject(NULL, _T("Local\\Wireshark child process cleanup")); + + if (!cjo_handle) { + g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Could not create child cleanup job object: %s", + win32strerror(GetLastError())); + return; + } + + JOBOBJECT_EXTENDED_LIMIT_INFORMATION cjo_jel_info = { 0 }; + cjo_jel_info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; + BOOL sijo_ret = SetInformationJobObject(cjo_handle, JobObjectExtendedLimitInformation, + &cjo_jel_info, sizeof(cjo_jel_info)); + if (!sijo_ret) { + g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Could not set child cleanup limits: %s", + win32strerror(GetLastError())); + } + } + + BOOL aptjo_ret = AssignProcessToJobObject(cjo_handle, child_handle); + if (!aptjo_ret) { + g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Could not assign child cleanup process: %s", + win32strerror(GetLastError())); + } +} + +BOOL win32_create_process(const char *application_name, const char *command_line, LPSECURITY_ATTRIBUTES process_attributes, LPSECURITY_ATTRIBUTES thread_attributes, BOOL inherit_handles, DWORD creation_flags, LPVOID environment, const char *current_directory, LPSTARTUPINFO startup_info, LPPROCESS_INFORMATION process_information) +{ + gunichar2 *wappname = NULL, *wcurrentdirectory = NULL; + gunichar2 *wcommandline = g_utf8_to_utf16(command_line, -1, NULL, NULL, NULL); + // CREATE_SUSPENDED: Suspend the child so that we can cleanly call + // AssignProcessToJobObject. + // CREATE_BREAKAWAY_FROM_JOB: The main application might be associated with + // a job, e.g. if we're running from ConEmu or Visual Studio. Break away + // from it so that we can cleanly call AssignProcessToJobObject on *our* job. + DWORD wcreationflags = creation_flags|CREATE_SUSPENDED|CREATE_BREAKAWAY_FROM_JOB; + + if (application_name) { + wappname = g_utf8_to_utf16(application_name, -1, NULL, NULL, NULL); + } + if (current_directory) { + wcurrentdirectory = g_utf8_to_utf16(current_directory, -1, NULL, NULL, NULL); + } + BOOL cp_res = CreateProcess(wappname, wcommandline, process_attributes, thread_attributes, + inherit_handles, wcreationflags, environment, wcurrentdirectory, startup_info, + process_information); + if (cp_res) { + win32_kill_child_on_exit(process_information->hProcess); + ResumeThread(process_information->hThread); + } + + g_free(wappname); + g_free(wcommandline); + g_free(wcurrentdirectory); + return cp_res; +} + /* * Editor modelines - http://www.wireshark.org/tools/modelines.html * * Local Variables: - * c-basic-offset: 2 + * c-basic-offset: 4 * tab-width: 8 * indent-tabs-mode: nil * End: * - * ex: set shiftwidth=2 tabstop=8 expandtab: - * :indentSize=2:tabSize=8:noTabs=true: + * ex: set shiftwidth=4 tabstop=8 expandtab: + * :indentSize=4:tabSize=8:noTabs=true: */ diff --git a/wsutil/win32-utils.h b/wsutil/win32-utils.h index 6a84dd0fa0..553d8f1921 100644 --- a/wsutil/win32-utils.h +++ b/wsutil/win32-utils.h @@ -64,6 +64,28 @@ const char * win32strerror(DWORD error); WS_DLL_PUBLIC const char * win32strexception(DWORD exception); +/** + * @brief ws_pipe_create_process Create a process and assign it to the main application + * job object so that it will be killed the the main application exits. + * @param application_name Application name. Will be converted to its UTF-16 equivalent or NULL. + * @param command_line Command line. Will be converted to its UTF-16 equivalent. + * @param process_attributes Same as CreateProcess. + * @param thread_attributes Same as CreateProcess. + * @param inherit_handles Same as CreateProcess. + * @param creation_flags Will be ORed with CREATE_SUSPENDED|CREATE_BREAKAWAY_FROM_JOB. + * @param environment Same as CreateProcess. + * @param current_directory Current directory. Will be converted to its UTF-16 equivalent or NULL. + * @param startup_info Same as CreateProcess. + * @param process_information Same as CreateProcess. + * @return + */ +WS_DLL_PUBLIC +BOOL win32_create_process(const char *application_name, const char *command_line, + LPSECURITY_ATTRIBUTES process_attributes, LPSECURITY_ATTRIBUTES thread_attributes, + BOOL inherit_handles, DWORD creation_flags, LPVOID environment, + const char *current_directory, LPSTARTUPINFO startup_info, LPPROCESS_INFORMATION process_information +); + #ifdef __cplusplus } #endif diff --git a/wsutil/ws_pipe.c b/wsutil/ws_pipe.c index d683b925f6..a18897adfb 100644 --- a/wsutil/ws_pipe.c +++ b/wsutil/ws_pipe.c @@ -33,54 +33,6 @@ #include "wsutil/filesystem.h" #include "wsutil/ws_pipe.h" -#ifdef _WIN32 -#include <tchar.h> -#include "wsutil/win32-utils.h" - -// This appears to be the closest equivalent to SIGPIPE on Windows. -// https://blogs.msdn.microsoft.com/oldnewthing/20131209-00/?p=2433 -// https://stackoverflow.com/a/53214/82195 -// We might want to make it public and use it for our other external -// processes. - -// CreateProcess flags. -// CREATE_NEW_CONSOLE: Don't interfere with the main application console. -// CREATE_SUSPENDED: Suspend the child so that we can cleanly call -// AssignProcessToJobObject. -// CREATE_BREAKAWAY_FROM_JOB: The main application might be associated with -// a job, e.g. if we're running from ConEmu or Visual Studio. Break away -// from it so that we can cleanly call AssignProcessToJobObject on *our* job. -#define WS_CP_JOB_FLAGS (CREATE_NEW_CONSOLE|CREATE_SUSPENDED|CREATE_BREAKAWAY_FROM_JOB) - -static void ws_pipe_kill_child_on_exit(ws_pipe_handle child_handle) { - static HANDLE cjo_handle = NULL; - if (!cjo_handle) { - cjo_handle = CreateJobObject(NULL, _T("Local\\Wireshark child process cleanup")); - - if (!cjo_handle) { - g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Could not create child cleanup job object: %s", - win32strerror(GetLastError())); - return; - } - - JOBOBJECT_EXTENDED_LIMIT_INFORMATION cjo_jel_info = { 0 }; - cjo_jel_info.BasicLimitInformation.LimitFlags = JOB_OBJECT_LIMIT_KILL_ON_JOB_CLOSE; - BOOL sijo_ret = SetInformationJobObject(cjo_handle, JobObjectExtendedLimitInformation, - &cjo_jel_info, sizeof(cjo_jel_info)); - if (!sijo_ret) { - g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Could not set child cleanup limits: %s", - win32strerror(GetLastError())); - } - } - - BOOL aptjo_ret = AssignProcessToJobObject(cjo_handle, child_handle); - if (!aptjo_ret) { - g_log(LOG_DOMAIN_CAPTURE, G_LOG_LEVEL_DEBUG, "Could not assign child cleanup process: %s", - win32strerror(GetLastError())); - } -} -#endif // _WIN32 - gboolean ws_pipe_spawn_sync(gchar *dirname, gchar *command, gint argc, gchar **args, gchar **command_output) { gboolean status = FALSE; @@ -94,7 +46,6 @@ gboolean ws_pipe_spawn_sync(gchar *dirname, gchar *command, gint argc, gchar **a GString *winargs = g_string_sized_new(200); gchar *quoted_arg; - gunichar2 *wcommandline; STARTUPINFO info; PROCESS_INFORMATION processInfo; @@ -158,8 +109,6 @@ gboolean ws_pipe_spawn_sync(gchar *dirname, gchar *command, gint argc, gchar **a g_free(quoted_arg); } - wcommandline = g_utf8_to_utf16(winargs->str, (glong)winargs->len, NULL, NULL, NULL); - memset(&processInfo, 0, sizeof(PROCESS_INFORMATION)); memset(&info, 0, sizeof(STARTUPINFO)); @@ -169,12 +118,10 @@ gboolean ws_pipe_spawn_sync(gchar *dirname, gchar *command, gint argc, gchar **a info.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW; info.wShowWindow = SW_HIDE; - if (CreateProcess(NULL, wcommandline, NULL, NULL, TRUE, WS_CP_JOB_FLAGS, NULL, NULL, &info, &processInfo)) + if (win32_create_process(NULL, winargs->str, NULL, NULL, TRUE, CREATE_NEW_CONSOLE, NULL, NULL, &info, &processInfo)) { gchar* buffer; - ws_pipe_kill_child_on_exit(processInfo.hProcess); - ResumeThread(processInfo.hThread); WaitForSingleObject(processInfo.hProcess, INFINITE); buffer = (gchar*)g_malloc(BUFFER_SIZE); status = ws_read_string_from_pipe(child_stdout_rd, buffer, BUFFER_SIZE); @@ -237,7 +184,6 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args) GString *winargs = g_string_sized_new(200); gchar *quoted_arg; - gunichar2 *wcommandline; STARTUPINFO info; PROCESS_INFORMATION processInfo; @@ -290,8 +236,6 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args) g_free(quoted_arg); } - wcommandline = g_utf8_to_utf16(winargs->str, (glong)winargs->len, NULL, NULL, NULL); - memset(&processInfo, 0, sizeof(PROCESS_INFORMATION)); memset(&info, 0, sizeof(STARTUPINFO)); @@ -302,10 +246,8 @@ GPid ws_pipe_spawn_async(ws_pipe_t *ws_pipe, GPtrArray *args) info.dwFlags = STARTF_USESTDHANDLES | STARTF_USESHOWWINDOW; info.wShowWindow = SW_HIDE; - if (CreateProcess(NULL, wcommandline, NULL, NULL, TRUE, WS_CP_JOB_FLAGS, NULL, NULL, &info, &processInfo)) + if (win32_create_process(NULL, winargs->str, NULL, NULL, TRUE, CREATE_NEW_CONSOLE, NULL, NULL, &info, &processInfo)) { - ws_pipe_kill_child_on_exit(processInfo.hProcess); - ResumeThread(processInfo.hThread); ws_pipe->stdin_fd = _open_osfhandle((intptr_t)(child_stdin_wr), _O_BINARY); ws_pipe->stdout_fd = _open_osfhandle((intptr_t)(child_stdout_rd), _O_BINARY); ws_pipe->stderr_fd = _open_osfhandle((intptr_t)(child_stderr_rd), _O_BINARY); diff --git a/wsutil/ws_pipe.h b/wsutil/ws_pipe.h index a5f37a1040..bd9fca63a3 100644 --- a/wsutil/ws_pipe.h +++ b/wsutil/ws_pipe.h @@ -73,6 +73,7 @@ static inline gboolean ws_pipe_valid(ws_pipe_t *ws_pipe) */ WS_DLL_PUBLIC GPid ws_pipe_spawn_async (ws_pipe_t * ws_pipe, GPtrArray * args ); +#ifdef _WIN32 /** * @brief Wait for a set of handles using WaitForMultipleObjects. Windows only. * @param pipe_handles An array of handles @@ -80,7 +81,6 @@ WS_DLL_PUBLIC GPid ws_pipe_spawn_async (ws_pipe_t * ws_pipe, GPtrArray * args ); * @param pid Child process PID. * @return TRUE on success or FALSE on failure. */ -#ifdef _WIN32 WS_DLL_PUBLIC gboolean ws_pipe_wait_for_pipe(HANDLE * pipe_handles, int num_pipe_handles, HANDLE pid); #endif |