dumpcap: fix memory leak in ringbuffer mode
'save_file' is used both for holding the -w command-line argument as well as the current filename that is being written. In ringbuffer mode, the former is already freed while the latter changes after rotation. Be sure to free all ringbuffer filenames on exit. Fixes test failures due to ASAN reporting memory leaks for: test_dumpcap_ringbuffer_filesize test_dumpcap_pcapng_single_in_multi_out test_dumpcap_pcapng_multi_in_multi_out test_dumpcap_ringbuffer_packets Change-Id: Ib817d8340275d7afa7e149dcfbbc59ed78293c34 Reviewed-on: https://code.wireshark.org/review/31739 Petri-Dish: Peter Wu <peter@lekensteyn.nl> Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman <a.broman58@gmail.com>
This commit is contained in:
parent
728183c27e
commit
0b632861e2
27
dumpcap.c
27
dumpcap.c
|
@ -1148,6 +1148,13 @@ exit_main(int status)
|
|||
|
||||
#endif /* _WIN32 */
|
||||
|
||||
if (ringbuf_is_initialized()) {
|
||||
/* save_file is managed by ringbuffer, be sure to release the memory and
|
||||
* avoid capture_opts_cleanup from double-freeing 'save_file'. */
|
||||
ringbuf_free();
|
||||
global_capture_opts.save_file = NULL;
|
||||
}
|
||||
|
||||
capture_opts_cleanup(&global_capture_opts);
|
||||
exit(status);
|
||||
}
|
||||
|
@ -3548,10 +3555,10 @@ capture_loop_open_output(capture_options *capture_opts, int *save_file_fd,
|
|||
(capture_opts->has_ring_num_files) ? capture_opts->ring_num_files : 0,
|
||||
capture_opts->group_read_access);
|
||||
|
||||
/* we need the ringbuf name */
|
||||
/* capfile_name is unused as the ringbuffer provides its own filename. */
|
||||
if (*save_file_fd != -1) {
|
||||
g_free(capfile_name);
|
||||
capfile_name = g_strdup(ringbuf_current_filename());
|
||||
capfile_name = NULL;
|
||||
}
|
||||
} else {
|
||||
/* Try to open/create the specified file for use as a capture buffer. */
|
||||
|
@ -3645,6 +3652,9 @@ capture_loop_open_output(capture_options *capture_opts, int *save_file_fd,
|
|||
"could not be opened: %s.", capfile_name, g_strerror(errno));
|
||||
} else {
|
||||
if (capture_opts->multi_files_on) {
|
||||
/* Ensures that the ringbuffer is not used. This ensures that
|
||||
* !ringbuf_is_initialized() is equivalent to
|
||||
* capture_opts->save_file not being part of ringbuffer. */
|
||||
ringbuf_error_cleanup();
|
||||
}
|
||||
|
||||
|
@ -3657,12 +3667,15 @@ capture_loop_open_output(capture_options *capture_opts, int *save_file_fd,
|
|||
return FALSE;
|
||||
}
|
||||
|
||||
if (capture_opts->save_file != NULL) {
|
||||
g_free(capture_opts->save_file);
|
||||
g_free(capture_opts->save_file);
|
||||
if (!is_tempfile && capture_opts->multi_files_on) {
|
||||
/* In ringbuffer mode, save_file points to a filename from ringbuffer.c.
|
||||
* capfile_name was already freed before. */
|
||||
capture_opts->save_file = (char *)ringbuf_current_filename();
|
||||
} else {
|
||||
/* capture_opts_cleanup will g_free(capture_opts->save_file). */
|
||||
capture_opts->save_file = capfile_name;
|
||||
}
|
||||
capture_opts->save_file = capfile_name;
|
||||
/* capture_opts.save_file is "g_free"ed later, which is equivalent to
|
||||
"g_free(capfile_name)". */
|
||||
|
||||
return TRUE;
|
||||
}
|
||||
|
|
12
ringbuffer.c
12
ringbuffer.c
|
@ -203,8 +203,18 @@ ringbuf_init(const char *capfile_name, guint num_files, gboolean group_read_acce
|
|||
}
|
||||
|
||||
|
||||
/*
|
||||
* Whether the ringbuf filenames are ready.
|
||||
* (Whether ringbuf_init is called and ringbuf_free is not called.)
|
||||
*/
|
||||
gboolean ringbuf_is_initialized(void)
|
||||
{
|
||||
return rb_data.files != NULL;
|
||||
}
|
||||
|
||||
const gchar *ringbuf_current_filename(void)
|
||||
{
|
||||
/* g_assert(ringbuf_is_initialized()); */
|
||||
return rb_data.files[rb_data.curr_file_num % rb_data.num_files].name;
|
||||
}
|
||||
|
||||
|
@ -359,7 +369,7 @@ ringbuf_error_cleanup(void)
|
|||
#endif /* HAVE_LIBPCAP */
|
||||
|
||||
/*
|
||||
* Editor modelines - http://www.wireshark.org/tools/modelines.html
|
||||
* Editor modelines - https://www.wireshark.org/tools/modelines.html
|
||||
*
|
||||
* Local Variables:
|
||||
* c-basic-offset: 2
|
||||
|
|
|
@ -24,6 +24,7 @@
|
|||
#define RINGBUFFER_WARN_NUM_FILES 65535
|
||||
|
||||
int ringbuf_init(const char *capture_name, guint num_files, gboolean group_read_access);
|
||||
gboolean ringbuf_is_initialized(void);
|
||||
const gchar *ringbuf_current_filename(void);
|
||||
FILE *ringbuf_init_libpcap_fdopen(int *err);
|
||||
gboolean ringbuf_switch_file(FILE **pdh, gchar **save_file, int *save_file_fd,
|
||||
|
@ -35,7 +36,7 @@ void ringbuf_error_cleanup(void);
|
|||
#endif /* ringbuffer.h */
|
||||
|
||||
/*
|
||||
* Editor modelines - http://www.wireshark.org/tools/modelines.html
|
||||
* Editor modelines - https://www.wireshark.org/tools/modelines.html
|
||||
*
|
||||
* Local Variables:
|
||||
* c-basic-offset: 2
|
||||
|
|
Loading…
Reference in New Issue