From affcc113b0e6e630e4166a45d6359d25cd1bc45f Mon Sep 17 00:00:00 2001 From: Michal Ruprich Date: Mon, 29 Jul 2024 09:34:08 +0200 Subject: [PATCH] Resolves: RHEL-49578 - Wireshark hangs if dumpcap returned unexpected messages in sync pipe --- ...shark-0009-sync-pipe-stderr-messages.patch | 612 ++++++++++++++++++ wireshark.spec | 6 +- 2 files changed, 617 insertions(+), 1 deletion(-) create mode 100644 wireshark-0009-sync-pipe-stderr-messages.patch diff --git a/wireshark-0009-sync-pipe-stderr-messages.patch b/wireshark-0009-sync-pipe-stderr-messages.patch new file mode 100644 index 0000000..e27f7d4 --- /dev/null +++ b/wireshark-0009-sync-pipe-stderr-messages.patch @@ -0,0 +1,612 @@ +From 4a454d8d626ade8804d2d492c796939d82b484b6 Mon Sep 17 00:00:00 2001 +From: John Thacker +Date: Thu, 28 Dec 2023 20:18:38 -0500 +Subject: [PATCH] dumpcap: Don't use fd 2 (stderr) for the sync pipe + +Don't use stderr for the sync pipe, because third party libraries +and functions sometimes pollute stderr with other information. + +Instead, pass the information necessary to dumpcap as a parameter +to the special capture child option -Z. + +On UN*X, that means passing the sync pipe write file descriptor, as +the child is created by fork + exec and shares the file descriptor table. + +On Windows, the child process does not share the file descriptor table, +but it does share the HANDLE value for inherited handles, so pass that +instead. (The HANDLE is a void* and technically 64 bit, but only the +lower 32 bits are used for inherited handles that can be shared + and it is permissible to truncate: +https://learn.microsoft.com/en-us/windows/win32/procthread/inheritance +https://learn.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication +https://learn.microsoft.com/en-us/windows/win32/WinProg64/rules-for-using-pointers +though perhaps in the future casting to an intptr_t makes more sense.) + +Move the special Windows named signal pipe to its own long option +instead of using the parameter from the capture child option. + +This means that we alter argv inside sync_pipe_open_command so change +the static functions and free argv there. Once glib 2.68 and later is +required a GStrvBuilder could be used instead. + +Fix #12222 +--- + capture/capture_sync.c | 105 ++++++++++++++++++++--------------------- + dumpcap.c | 85 +++++++++++++++++++++++---------- + 2 files changed, 112 insertions(+), 78 deletions(-) + +diff --git a/capture/capture_sync.c b/capture/capture_sync.c +index 5c70baa6dfb..aa29a0b7ff7 100644 +--- a/capture/capture_sync.c ++++ b/capture/capture_sync.c +@@ -115,14 +115,6 @@ static ssize_t pipe_read_block(GIOChannel *pipe_io, char *indicator, int len, ch + + static void (*fetch_dumpcap_pid)(ws_process_id) = NULL; + +-static void free_argv(char** argv, int argc) +-{ +- int i; +- for (i = 0; i < argc; i++) +- g_free(argv[i]); +- g_free(argv); +-} +- + void + capture_session_init(capture_session *cap_session, capture_file *cf, + new_file_fn new_file, new_packets_fn new_packets, +@@ -202,6 +194,8 @@ void capture_process_finished(capture_session *cap_session) + + /* Append an arg (realloc) to an argc/argv array */ + /* (add a string pointer to a NULL-terminated array of string pointers) */ ++/* XXX: For glib >= 2.68 we could use a GStrvBuilder. ++ */ + static char ** + sync_pipe_add_arg(char **args, int *argc, const char *arg) + { +@@ -277,12 +271,12 @@ pipe_io_cb(GIOChannel *pipe_io, GIOCondition condition _U_, void * user_data) + #define PIPE_BUF_SIZE 5120 + static int + #ifdef _WIN32 +-sync_pipe_open_command(char* const argv[], int *data_read_fd, ++sync_pipe_open_command(char **argv, int *data_read_fd, + GIOChannel **message_read_io, int *signal_write_fd, + ws_process_id *fork_child, GArray *ifaces, + char **msg, void(*update_cb)(void)) + #else +-sync_pipe_open_command(char* const argv[], int *data_read_fd, ++sync_pipe_open_command(char **argv, int *data_read_fd, + GIOChannel **message_read_io, int *signal_write_fd _U_, + ws_process_id *fork_child, GArray *ifaces _U_, + char **msg, void(*update_cb)(void)) +@@ -290,6 +284,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + { + enum PIPES { PIPE_READ, PIPE_WRITE }; /* Constants 0 and 1 for PIPE_READ and PIPE_WRITE */ + int message_read_fd = -1; ++ char sync_id[ARGV_NUMBER_LEN]; + #ifdef _WIN32 + HANDLE sync_pipe[2]; /* pipe used to send messages from child to parent */ + HANDLE data_pipe[2]; /* pipe used to send data from child to parent */ +@@ -320,6 +315,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + + if (!msg) { + /* We can't return anything */ ++ g_strfreev(argv); + #ifdef _WIN32 + g_string_free(args, true); + #endif +@@ -338,6 +334,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + /* Couldn't create the message pipe between parent and child. */ + *msg = ws_strdup_printf("Couldn't create sync pipe: %s", + win32strerror(GetLastError())); ++ g_strfreev(argv); + return -1; + } + +@@ -351,6 +348,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + message_read_fd = _open_osfhandle( (intptr_t) sync_pipe[PIPE_READ], _O_BINARY); + if (message_read_fd == -1) { + *msg = ws_strdup_printf("Couldn't get C file handle for message read pipe: %s", g_strerror(errno)); ++ g_strfreev(argv); + CloseHandle(sync_pipe[PIPE_READ]); + CloseHandle(sync_pipe[PIPE_WRITE]); + return -1; +@@ -363,6 +361,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + /* Couldn't create the message pipe between parent and child. */ + *msg = ws_strdup_printf("Couldn't create data pipe: %s", + win32strerror(GetLastError())); ++ g_strfreev(argv); + ws_close(message_read_fd); /* Should close sync_pipe[PIPE_READ] */ + CloseHandle(sync_pipe[PIPE_WRITE]); + return -1; +@@ -378,6 +377,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + *data_read_fd = _open_osfhandle( (intptr_t) data_pipe[PIPE_READ], _O_BINARY); + if (*data_read_fd == -1) { + *msg = ws_strdup_printf("Couldn't get C file handle for data read pipe: %s", g_strerror(errno)); ++ g_strfreev(argv); + CloseHandle(data_pipe[PIPE_READ]); + CloseHandle(data_pipe[PIPE_WRITE]); + ws_close(message_read_fd); /* Should close sync_pipe[PIPE_READ] */ +@@ -398,6 +398,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + /* Couldn't create the signal pipe between parent and child. */ + *msg = ws_strdup_printf("Couldn't create signal pipe: %s", + win32strerror(GetLastError())); ++ g_strfreev(argv); + ws_close(message_read_fd); /* Should close sync_pipe[PIPE_READ] */ + CloseHandle(sync_pipe[PIPE_WRITE]); + return -1; +@@ -414,6 +415,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + if (signal_pipe_write_fd == -1) { + /* Couldn't create the pipe between parent and child. */ + *msg = ws_strdup_printf("Couldn't get C file handle for sync pipe: %s", g_strerror(errno)); ++ g_strfreev(argv); + ws_close(message_read_fd); /* Should close sync_pipe[PIPE_READ] */ + CloseHandle(sync_pipe[PIPE_WRITE]); + CloseHandle(signal_pipe); +@@ -439,7 +441,25 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + si.hStdInput = NULL; /* handle for named pipe*/ + si.hStdOutput = data_pipe[PIPE_WRITE]; + } +- si.hStdError = sync_pipe[PIPE_WRITE]; ++ si.hStdError = GetStdHandle(STD_ERROR_HANDLE); ++ ++ /* On Windows, "[a]n inherited handle refers to the same object in the child ++ * process as it does in the parent process. It also has the same value." ++ * https://learn.microsoft.com/en-us/windows/win32/procthread/inheritance ++ * When converted to a file descriptor (via _open_osfhandle), the fd ++ * value is not necessarily the same in the two processes, but the handle ++ * value can be shared. ++ * A HANDLE is a void* though "64-bit versions of Windows use 32-bit handles ++ * for interoperability... only the lower 32 bits are significant, so it is ++ * safe to truncate the handle... or sign-extend the handle" ++ * https://learn.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication ++ * So it should be fine to call PtrToLong instead of casting to intptr_t. ++ * https://learn.microsoft.com/en-us/windows/win32/WinProg64/rules-for-using-pointers ++ */ ++ int argc = g_strv_length(argv); ++ argv = sync_pipe_add_arg(argv, &argc, "-Z"); ++ snprintf(sync_id, ARGV_NUMBER_LEN, "%ld", PtrToLong(sync_pipe[PIPE_WRITE])); ++ argv = sync_pipe_add_arg(argv, &argc, sync_id); + #endif + + if (ifaces) { +@@ -458,7 +478,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + if (si.hStdOutput && (si.hStdOutput != si.hStdInput)) { + handles[i_handles++] = si.hStdOutput; + } +- handles[i_handles++] = si.hStdError; ++ handles[i_handles++] = sync_pipe[PIPE_WRITE]; + if (ifaces) { + for (j = 0; j < ifaces->len; j++) { + interface_opts = &g_array_index(ifaces, interface_options, j); +@@ -491,6 +511,7 @@ 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_strfreev(argv); + g_string_free(args, true); + g_free(handles); + return -1; +@@ -498,6 +519,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + *fork_child = pi.hProcess; + /* We may need to store this and close it later */ + CloseHandle(pi.hThread); ++ g_strfreev(argv); + g_string_free(args, true); + g_free(handles); + +@@ -509,6 +531,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + if (pipe(sync_pipe) < 0) { + /* Couldn't create the message pipe between parent and child. */ + *msg = ws_strdup_printf("Couldn't create sync pipe: %s", g_strerror(errno)); ++ g_strfreev(argv); + return -1; + } + +@@ -517,6 +540,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + if (pipe(data_pipe) < 0) { + /* Couldn't create the data pipe between parent and child. */ + *msg = ws_strdup_printf("Couldn't create data pipe: %s", g_strerror(errno)); ++ g_strfreev(argv); + ws_close(sync_pipe[PIPE_READ]); + ws_close(sync_pipe[PIPE_WRITE]); + return -1; +@@ -533,11 +557,16 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + ws_close(data_pipe[PIPE_READ]); + ws_close(data_pipe[PIPE_WRITE]); + } +- dup2(sync_pipe[PIPE_WRITE], 2); + ws_close(sync_pipe[PIPE_READ]); +- ws_close(sync_pipe[PIPE_WRITE]); ++ /* dumpcap should be running in capture child mode (hidden feature) */ ++#ifndef DEBUG_CHILD ++ int argc = g_strv_length(argv); ++ argv = sync_pipe_add_arg(argv, &argc, "-Z"); ++ snprintf(sync_id, ARGV_NUMBER_LEN, "%d", sync_pipe[PIPE_WRITE]); ++ argv = sync_pipe_add_arg(argv, &argc, sync_id); ++#endif + execv(argv[0], argv); +- sync_pipe_write_int_msg(2, SP_EXEC_FAILED, errno); ++ sync_pipe_write_int_msg(sync_pipe[PIPE_WRITE], SP_EXEC_FAILED, errno); + + /* Exit with "_exit()", so that we don't close the connection + to the X server (and cause stuff buffered up by our parent but +@@ -549,6 +578,8 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + _exit(1); + } + ++ g_strfreev(argv); ++ + if (fetch_dumpcap_pid && *fork_child > 0) + fetch_dumpcap_pid(*fork_child); + +@@ -556,6 +587,7 @@ sync_pipe_open_command(char* const argv[], int *data_read_fd, + *data_read_fd = data_pipe[PIPE_READ]; + } + message_read_fd = sync_pipe[PIPE_READ]; ++ + #endif + + /* Parent process - read messages from the child process over the +@@ -863,14 +895,12 @@ sync_pipe_start(capture_options *capture_opts, GPtrArray *capture_comments, + } + } + +- /* dumpcap should be running in capture child mode (hidden feature) */ + #ifndef DEBUG_CHILD +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); + #ifdef _WIN32 ++ /* pass process id to dumpcap for named signal pipe */ ++ argv = sync_pipe_add_arg(argv, &argc, "--signal-pipe"); + snprintf(control_id, ARGV_NUMBER_LEN, "%ld", GetCurrentProcessId()); + argv = sync_pipe_add_arg(argv, &argc, control_id); +-#else +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); + #endif + #endif + +@@ -899,13 +929,11 @@ sync_pipe_start(capture_options *capture_opts, GPtrArray *capture_comments, + if (ret == -1) { + report_failure("%s", msg); + g_free(msg); +- free_argv(argv, argc); + return false; + } + + /* Parent process - read messages from the child process over the + sync pipe. */ +- free_argv(argv, argc); + + cap_session->fork_child_status = 0; + cap_session->cap_data_info = cap_data; +@@ -964,7 +992,7 @@ sync_pipe_close_command(int *data_read_fd, GIOChannel *message_read_io, + /* XXX - assumes PIPE_BUF_SIZE > SP_MAX_MSG_LEN */ + #define PIPE_BUF_SIZE 5120 + static int +-sync_pipe_run_command_actual(char* const argv[], char **data, char **primary_msg, ++sync_pipe_run_command_actual(char **argv, char **data, char **primary_msg, + char **secondary_msg, void(*update_cb)(void)) + { + char *msg; +@@ -1176,7 +1204,7 @@ sync_pipe_run_command_actual(char* const argv[], char **data, char **primary_msg + * redirects to sync_pipe_run_command_actual() + */ + static int +-sync_pipe_run_command(char* const argv[], char **data, char **primary_msg, ++sync_pipe_run_command(char **argv, char **data, char **primary_msg, + char **secondary_msg, void (*update_cb)(void)) + { + int ret, i; +@@ -1241,22 +1269,14 @@ sync_interface_set_80211_chan(const char *iface, const char *freq, const char *t + *primary_msg = g_strdup("Out of mem."); + *secondary_msg = NULL; + *data = NULL; +- free_argv(argv, argc); + return -1; + } + + argv = sync_pipe_add_arg(argv, &argc, "-k"); + argv = sync_pipe_add_arg(argv, &argc, opt); + +-#ifndef DEBUG_CHILD +- /* Run dumpcap in capture child mode */ +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); +-#endif +- + ret = sync_pipe_run_command(argv, data, primary_msg, secondary_msg, update_cb); + g_free(opt); +- free_argv(argv, argc); + return ret; + } + +@@ -1294,13 +1314,7 @@ sync_interface_list_open(char **data, char **primary_msg, + /* Ask for the interface list */ + argv = sync_pipe_add_arg(argv, &argc, "-D"); + +-#ifndef DEBUG_CHILD +- /* Run dumpcap in capture child mode */ +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); +-#endif + ret = sync_pipe_run_command(argv, data, primary_msg, secondary_msg, update_cb); +- free_argv(argv, argc); + return ret; + } + +@@ -1348,13 +1362,7 @@ sync_if_capabilities_open(const char *ifname, bool monitor_mode, const char* aut + argv = sync_pipe_add_arg(argv, &argc, auth); + } + +-#ifndef DEBUG_CHILD +- /* Run dumpcap in capture child mode */ +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); +-#endif + ret = sync_pipe_run_command(argv, data, primary_msg, secondary_msg, update_cb); +- free_argv(argv, argc); + return ret; + } + +@@ -1449,20 +1451,17 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d + argv = sync_pipe_add_arg(argv, &argc, "-S"); + + #ifndef DEBUG_CHILD +- argv = sync_pipe_add_arg(argv, &argc, "-Z"); + #ifdef _WIN32 ++ argv = sync_pipe_add_arg(argv, &argc, "--signal-pipe"); + ret = create_dummy_signal_pipe(msg); + if (ret == -1) { + return -1; + } + argv = sync_pipe_add_arg(argv, &argc, dummy_control_id); +-#else +- argv = sync_pipe_add_arg(argv, &argc, SIGNAL_PIPE_CTRL_ID_NONE); + #endif + #endif + ret = sync_pipe_open_command(argv, data_read_fd, &message_read_io, NULL, + fork_child, NULL, msg, update_cb); +- free_argv(argv, argc); + if (ret == -1) { + return -1; + } +diff --git a/dumpcap.c b/dumpcap.c +index b5a5423..533aa96 100644 +--- a/dumpcap.c ++++ b/dumpcap.c +@@ -130,6 +130,7 @@ static gchar *sig_pipe_name = NULL; + static HANDLE sig_pipe_handle = NULL; + static gboolean signal_pipe_check_running(void); + #endif ++static int sync_pipe_fd = 2; + + #ifdef ENABLE_ASAN + /* This has public visibility so that if compiled with shared libasan (the +@@ -562,7 +563,7 @@ dumpcap_cmdarg_err(const char *fmt, va_list ap) + gchar *msg; + /* Generate a 'special format' message back to parent */ + msg = ws_strdup_vprintf(fmt, ap); +- sync_pipe_write_errmsgs_to_parent(2, msg, ""); ++ sync_pipe_write_errmsgs_to_parent(sync_pipe_fd, msg, ""); + g_free(msg); + } else { + fprintf(stderr, "dumpcap: "); +@@ -582,7 +583,7 @@ dumpcap_cmdarg_err_cont(const char *fmt, va_list ap) + if (capture_child) { + gchar *msg; + msg = ws_strdup_vprintf(fmt, ap); +- sync_pipe_write_errmsgs_to_parent(2, msg, ""); ++ sync_pipe_write_errmsgs_to_parent(sync_pipe_fd, msg, ""); + g_free(msg); + } else { + vfprintf(stderr, fmt, ap); +@@ -1007,7 +1008,7 @@ show_filter_code(capture_options *capture_opts) + #endif + if (capture_child) { + /* Let our parent know we succeeded. */ +- sync_pipe_write_string_msg(2, SP_SUCCESS, NULL); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_SUCCESS, NULL); + } + return TRUE; + } +@@ -1029,7 +1030,7 @@ print_machine_readable_interfaces(GList *if_list) + + if (capture_child) { + /* Let our parent know we succeeded. */ +- sync_pipe_write_string_msg(2, SP_SUCCESS, NULL); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_SUCCESS, NULL); + } + + i = 1; /* Interface id number */ +@@ -1096,7 +1097,7 @@ print_machine_readable_if_capabilities(if_capabilities_t *caps, int queries) + + if (capture_child) { + /* Let our parent know we succeeded. */ +- sync_pipe_write_string_msg(2, SP_SUCCESS, NULL); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_SUCCESS, NULL); + } + + if (queries & CAPS_QUERY_LINK_TYPES) { +@@ -1197,7 +1198,7 @@ print_statistics_loop(gboolean machine_readable) + + if (capture_child) { + /* Let our parent know we succeeded. */ +- sync_pipe_write_string_msg(2, SP_SUCCESS, NULL); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_SUCCESS, NULL); + } + + if (!machine_readable) { +@@ -4985,7 +4986,7 @@ capture_loop_write_pcapng_cb(capture_src *pcap_src, const pcapng_block_header_t + ws_info("Sending SP_FILE on first SHB"); + #endif + /* SHB is now ready for capture parent to read on SP_FILE message */ +- sync_pipe_write_string_msg(2, SP_FILE, report_capture_filename); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_FILE, report_capture_filename); + report_capture_filename = NULL; + } + } +@@ -5214,7 +5215,7 @@ set_80211_channel(const char *iface, const char *opt) + } + + if (capture_child) +- sync_pipe_write_string_msg(2, SP_SUCCESS, NULL); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_SUCCESS, NULL); + + out: + g_strfreev(options); +@@ -5238,6 +5239,9 @@ gather_dumpcap_runtime_info(feature_list l) + #define LONGOPT_IFNAME LONGOPT_BASE_APPLICATION+1 + #define LONGOPT_IFDESCR LONGOPT_BASE_APPLICATION+2 + #define LONGOPT_CAPTURE_COMMENT LONGOPT_BASE_APPLICATION+3 ++#ifdef _WIN32 ++#define LONGOPT_SIGNAL_PIPE LONGOPT_BASE_APPLICATION+4 ++#endif + + /* And now our feature presentation... [ fade to music ] */ + int +@@ -5252,6 +5256,9 @@ main(int argc, char *argv[]) + {"ifname", ws_required_argument, NULL, LONGOPT_IFNAME}, + {"ifdescr", ws_required_argument, NULL, LONGOPT_IFDESCR}, + {"capture-comment", ws_required_argument, NULL, LONGOPT_CAPTURE_COMMENT}, ++#ifdef _WIN32 ++ {"signal-pipe", ws_required_argument, NULL, LONGOPT_SIGNAL_PIPE}, ++#endif + {0, 0, 0, 0 } + }; + +@@ -5308,10 +5315,31 @@ main(int argc, char *argv[]) + if (strcmp("-Z", argv[i]) == 0) { + capture_child = TRUE; + machine_readable = TRUE; /* request machine-readable output */ ++ i++; ++ if (i >= argc) { ++ exit_main(1); ++ } ++ ++ if (strcmp(argv[i], SIGNAL_PIPE_CTRL_ID_NONE) != 0) { ++ // get_positive_int calls cmdarg_err ++ if (!ws_strtoi(argv[i], NULL, &sync_pipe_fd) || sync_pipe_fd <= 0) { ++ exit_main(1); ++ } + #ifdef _WIN32 +- /* set output pipe to binary mode, to avoid ugly text conversions */ +- _setmode(2, O_BINARY); ++ /* On UN*X the fd is the same when we fork + exec. ++ * On Windows the HANDLE value is the same for inherited ++ * handles in the child process and the parent, although ++ * not necessarily the fd value from _open_osfhandle. ++ * https://learn.microsoft.com/en-us/windows/win32/procthread/inheritance ++ * Also, "64-bit versions of Windows use 32-bit handles for ++ * interoperability... only the lower 32 bits are significant, ++ * so it is safe to truncate... or sign-extend the handle." ++ * https://learn.microsoft.com/en-us/windows/win32/winprog64/interprocess-communication ++ */ ++ /* set output pipe to binary mode, avoid ugly text conversions */ ++ sync_pipe_fd = _open_osfhandle( (intptr_t) sync_pipe_fd, _O_BINARY); + #endif ++ } + } + } + +@@ -5628,9 +5656,17 @@ main(int argc, char *argv[]) + break; + case 'Z': + capture_child = TRUE; ++ /* ++ * Handled above ++ */ ++ break; + #ifdef _WIN32 +- /* set output pipe to binary mode, to avoid ugly text conversions */ +- _setmode(2, O_BINARY); ++ case LONGOPT_SIGNAL_PIPE: ++ if (!capture_child) { ++ /* We have already checked for -Z at the very beginning. */ ++ cmdarg_err("--signal-pipe may only be specified with -Z"); ++ exit_main(1); ++ } + /* + * ws_optarg = the control ID, aka the PPID, currently used for the + * signal pipe name. +@@ -5646,9 +5682,8 @@ main(int argc, char *argv[]) + exit_main(1); + } + } +-#endif + break; +- ++#endif + case 'q': /* Quiet */ + quiet = TRUE; + break; +@@ -5885,7 +5920,7 @@ main(int argc, char *argv[]) + char *error_msg = ws_strdup_printf("The capabilities of the capture device " + "\"%s\" could not be obtained (%s)", + interface_opts->name, open_status_str); +- sync_pipe_write_errmsgs_to_parent(2, error_msg, ++ sync_pipe_write_errmsgs_to_parent(sync_pipe_fd, error_msg, + get_pcap_failure_secondary_error_message(open_status, open_status_str)); + g_free(error_msg); + } +@@ -6032,7 +6067,7 @@ dumpcap_log_writer(const char *domain, enum ws_log_level level, + #endif + if (capture_child) { + gchar *msg = ws_strdup_vprintf(user_format, user_ap); +- sync_pipe_write_errmsgs_to_parent(2, msg, ""); ++ sync_pipe_write_errmsgs_to_parent(sync_pipe_fd, msg, ""); + g_free(msg); + } else { + ws_log_console_writer(domain, level, file, line, func, mft, user_format, user_ap); +@@ -6052,7 +6087,7 @@ dumpcap_log_writer(const char *domain, enum ws_log_level level, + /* to parent especially formatted if dumpcap running as child. */ + if (capture_child) { + gchar *msg = ws_strdup_vprintf(user_format, user_ap); +- sync_pipe_write_errmsgs_to_parent(2, msg, ""); ++ sync_pipe_write_errmsgs_to_parent(sync_pipe_fd, msg, ""); + g_free(msg); + } else if(ws_log_msg_is_active(domain, level)) { + ws_log_console_writer(domain, level, file, line, func, mft, user_format, user_ap); +@@ -6071,7 +6106,7 @@ report_packet_count(unsigned int packet_count) + + if (capture_child) { + ws_debug("Packets: %u", packet_count); +- sync_pipe_write_uint_msg(2, SP_PACKET_COUNT, packet_count); ++ sync_pipe_write_uint_msg(sync_pipe_fd, SP_PACKET_COUNT, packet_count); + } else { + count += packet_count; + fprintf(stderr, "\rPackets: %u ", count); +@@ -6092,7 +6127,7 @@ report_new_capture_file(const char *filename) + #endif + report_capture_filename = filename; + } else { +- sync_pipe_write_string_msg(2, SP_FILE, filename); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_FILE, filename); + } + } else { + #ifdef SIGINFO +@@ -6132,7 +6167,7 @@ report_cfilter_error(capture_options *capture_opts, guint i, const char *errmsg) + if (capture_child) { + snprintf(tmp, sizeof(tmp), "%u:%s", i, errmsg); + ws_debug("Capture filter error: %s", errmsg); +- sync_pipe_write_string_msg(2, SP_BAD_FILTER, tmp); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_BAD_FILTER, tmp); + } else { + /* + * clopts_step_invalid_capfilter in test/suite-clopts.sh MUST match +@@ -6155,7 +6190,7 @@ report_capture_error(const char *error_msg, const char *secondary_error_msg) + if (capture_child) { + ws_debug("Primary Error: %s", error_msg); + ws_debug("Secondary Error: %s", secondary_error_msg); +- sync_pipe_write_errmsgs_to_parent(2, error_msg, secondary_error_msg); ++ sync_pipe_write_errmsgs_to_parent(sync_pipe_fd, error_msg, secondary_error_msg); + } else { + cmdarg_err("%s", error_msg); + if (secondary_error_msg[0] != '\0') +@@ -6173,7 +6208,7 @@ report_packet_drops(guint32 received, guint32 pcap_drops, guint32 drops, guint32 + + ws_debug("Packets received/dropped on interface '%s': %u/%u (pcap:%u/dumpcap:%u/flushed:%u/ps_ifdrop:%u)", + name, received, total_drops, pcap_drops, drops, flushed, ps_ifdrop); +- sync_pipe_write_string_msg(2, SP_DROPS, tmp); ++ sync_pipe_write_string_msg(sync_pipe_fd, SP_DROPS, tmp); + g_free(tmp); + } else { + fprintf(stderr, diff --git a/wireshark.spec b/wireshark.spec index 06c607a..51e182a 100644 --- a/wireshark.spec +++ b/wireshark.spec @@ -6,7 +6,7 @@ Summary: Network traffic analyzer Name: wireshark Version: 4.2.5 -Release: 2%{?dist} +Release: 3%{?dist} Epoch: 1 License: BSD-1-Clause AND BSD-2-Clause AND BSD-3-Clause AND MIT AND GPL-2.0-or-later AND LGPL-2.0-or-later AND Zlib AND ISC AND (BSD-3-Clause OR GPL-2.0-only) AND (GPL-2.0-or-later AND Zlib) Url: http://www.wireshark.org/ @@ -28,6 +28,7 @@ Patch5: wireshark-0005-Fix-paths-in-a-wireshark.desktop-file.patch Patch6: wireshark-0006-Move-tmp-to-var-tmp.patch Patch7: wireshark-0007-cmakelists.patch Patch8: wireshark-0008-pkgconfig.patch +Patch9: wireshark-0009-sync-pipe-stderr-messages.patch #install tshark together with wireshark GUI Requires: %{name}-cli = %{epoch}:%{version}-%{release} @@ -280,6 +281,9 @@ fi %{_libdir}/pkgconfig/%{name}.pc %changelog +* Mon Jul 29 2024 Michal Ruprich - 1:4.2.5-3 +- Resolves: RHEL-49578 - Wireshark hangs if dumpcap returned unexpected messages in sync pipe + * Mon Jun 24 2024 Troy Dawson - 1:4.2.5-2 - Bump release for June 2024 mass rebuild