Skip to content

Commit

Permalink
capture-sync: Fix deadlock with lots of interfaces.
Browse files Browse the repository at this point in the history
This code is still fragile, it should not deadlock even if there
is too much to read, but this at least hacks around the problem.

To avoid huge stacks, allocate the buffer on the heap.

Fixes wireshark hang with about 50 interfaces...

Signed-off-by: Ben Greear <greearb@candelatech.com>
  • Loading branch information
greearb authored and AndersBroman committed Jul 11, 2024
1 parent 1fd7c1d commit b12141f
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 4 deletions.
18 changes: 15 additions & 3 deletions capture/capture_sync.c
Original file line number Diff line number Diff line change
Expand Up @@ -1034,7 +1034,7 @@ sync_pipe_run_command_actual(char **argv, char **data, char **primary_msg,
GIOChannel *sync_pipe_read_io;
ws_process_id fork_child;
char *wait_msg;
char buffer[PIPE_BUF_SIZE+1] = {0};
char *buffer = g_malloc(PIPE_BUF_SIZE + 1);
ssize_t nread;
char indicator;
int32_t exec_errno = 0;
Expand All @@ -1052,6 +1052,7 @@ sync_pipe_run_command_actual(char **argv, char **data, char **primary_msg,
*primary_msg = msg;
*secondary_msg = NULL;
*data = NULL;
g_free(buffer);
return -1;
}

Expand Down Expand Up @@ -1097,6 +1098,7 @@ sync_pipe_run_command_actual(char **argv, char **data, char **primary_msg,
}
*secondary_msg = NULL;
*data = NULL;
g_free(buffer);

return -1;
}
Expand Down Expand Up @@ -1242,6 +1244,7 @@ sync_pipe_run_command_actual(char **argv, char **data, char **primary_msg,
}
} while (indicator != SP_SUCCESS && ret != -1);

g_free(buffer);
return ret;
}

Expand Down Expand Up @@ -1471,7 +1474,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d
int ret;
GIOChannel *message_read_io;
char *wait_msg;
char buffer[PIPE_BUF_SIZE+1] = {0};
char *buffer = g_malloc(PIPE_BUF_SIZE + 1);
ssize_t nread;
char indicator;
int32_t exec_errno = 0;
Expand All @@ -1487,6 +1490,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d

if (!argv) {
*msg = g_strdup("We don't know where to find dumpcap.");
g_free(buffer);
return -1;
}

Expand All @@ -1504,6 +1508,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d
argv = sync_pipe_add_arg(argv, &argc, "--signal-pipe");
ret = create_dummy_signal_pipe(msg);
if (ret == -1) {
g_free(buffer);
return -1;
}
argv = sync_pipe_add_arg(argv, &argc, dummy_control_id);
Expand All @@ -1512,6 +1517,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d
ret = sync_pipe_open_command(argv, data_read_fd, &message_read_io, NULL,
fork_child, NULL, msg, update_cb);
if (ret == -1) {
g_free(buffer);
return -1;
}

Expand Down Expand Up @@ -1556,6 +1562,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d
*msg = combined_msg;
}
}
g_free(buffer);
return -1;
}

Expand Down Expand Up @@ -1626,6 +1633,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d
*msg = g_strdup(primary_msg_text);
ret = -1;
}
g_free(buffer);
return ret;

case SP_LOG_MSG:
Expand Down Expand Up @@ -1674,6 +1682,7 @@ sync_interface_stats_open(int *data_read_fd, ws_process_id *fork_child, char **d
}
} while (indicator != SP_SUCCESS && ret != -1);

g_free(buffer);
return ret;
}

Expand Down Expand Up @@ -1859,7 +1868,7 @@ static gboolean
sync_pipe_input_cb(GIOChannel *pipe_io, capture_session *cap_session)
{
int ret;
char buffer[SP_MAX_MSG_LEN+1] = {0};
char *buffer = g_malloc(SP_MAX_MSG_LEN + 1);
ssize_t nread;
char indicator;
int32_t exec_errno = 0;
Expand Down Expand Up @@ -1918,6 +1927,7 @@ sync_pipe_input_cb(GIOChannel *pipe_io, capture_session *cap_session)
} else {
extcap_request_stop(cap_session);
}
g_free(buffer);
return false;
}

Expand All @@ -1944,6 +1954,7 @@ sync_pipe_input_cb(GIOChannel *pipe_io, capture_session *cap_session)
"standard output", as the capture file. */
sync_pipe_stop(cap_session);
cap_session->closed(cap_session, NULL);
g_free(buffer);
return false;
}
break;
Expand Down Expand Up @@ -2019,6 +2030,7 @@ sync_pipe_input_cb(GIOChannel *pipe_io, capture_session *cap_session)
break;
}

g_free(buffer);
return true;
}

Expand Down
2 changes: 1 addition & 1 deletion sync_pipe.h
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
* 4096 is a typical PIPE_BUF size for atomic writes, but we should have
* only one writer and one reader so that shouldn't be an issue.
*/
#define SP_MAX_MSG_LEN 24572
#define SP_MAX_MSG_LEN (512 * 1000)

/*
* Indications sent out on the sync pipe (from child to parent).
Expand Down

0 comments on commit b12141f

Please sign in to comment.