From 6e12643f198500245ee6eff999804e468481a7b3 Mon Sep 17 00:00:00 2001 From: David Perry Date: Wed, 4 Aug 2021 13:18:46 -0400 Subject: [PATCH] [#17478] free blocks in more places Bug 17478 was caused by `wtap_rec.block` being allocated for each packet, but not freed when it was done being used -- typically at the end of a loop. Rather than requiring each caller of `wtap_read()` to know to free a member of `rec`, I added a new function `wtap_rec_reset()` for a slightly cleaner API. Added calls to it everywhere that seemed to make sense. Fixes #17478 --- capinfos.c | 1 + debian/libwiretap0.symbols | 1 + editcap.c | 3 +++ file.c | 5 +++++ reordercap.c | 2 ++ sharkd.c | 4 ++++ tshark.c | 4 ++++ ui/capture.c | 1 + ui/file_dialog.c | 1 + wiretap/merge.c | 1 + wiretap/wtap.c | 11 +++++++++-- wiretap/wtap.h | 4 ++++ 12 files changed, 36 insertions(+), 2 deletions(-) diff --git a/capinfos.c b/capinfos.c index 495554b699..6b33f9d263 100644 --- a/capinfos.c +++ b/capinfos.c @@ -1324,6 +1324,7 @@ process_cap_file(const char *filename, gboolean need_separator) } } + wtap_rec_reset(&rec); } /* while */ wtap_rec_cleanup(&rec); ws_buffer_free(&buf); diff --git a/debian/libwiretap0.symbols b/debian/libwiretap0.symbols index 7e1a6b12e8..45db4ab48c 100644 --- a/debian/libwiretap0.symbols +++ b/debian/libwiretap0.symbols @@ -154,6 +154,7 @@ libwiretap.so.0 libwiretap0 #MINVER# wtap_read_so_far@Base 1.9.1 wtap_rec_cleanup@Base 2.5.1 wtap_rec_init@Base 2.5.1 + wtap_rec_reset@Base 3.5.0 wtap_register_encap_type@Base 1.9.1 wtap_register_file_type_extension@Base 1.12.0~rc1 wtap_register_file_type_subtype@Base 3.5.0 diff --git a/editcap.c b/editcap.c index 145504db7a..f7feb51582 100644 --- a/editcap.c +++ b/editcap.c @@ -1155,6 +1155,7 @@ main(int argc, char *argv[]) unsigned int seed = 0; cmdarg_err_init(editcap_cmdarg_err, editcap_cmdarg_err_cont); + memset(&read_rec, 0, sizeof *rec); /* Initialize log handler early so we can have proper logging during startup. */ ws_log_init("editcap", vcmdarg_err); @@ -2216,6 +2217,7 @@ main(int argc, char *argv[]) written_count++; } count++; + wtap_rec_reset(&read_rec); } wtap_rec_cleanup(&read_rec); ws_buffer_free(&read_buf); @@ -2285,6 +2287,7 @@ clean_exit: wtap_dump_params_cleanup(¶ms); if (wth != NULL) wtap_close(wth); + wtap_rec_reset(&read_rec); wtap_cleanup(); free_progdirs(); if (capture_comments != NULL) { diff --git a/file.c b/file.c index 1464eb859f..29b6114d68 100644 --- a/file.c +++ b/file.c @@ -653,6 +653,7 @@ cf_read(capture_file *cf, gboolean reloading) break; } read_record(cf, &rec, &buf, dfcode, &edt, cinfo, data_offset); + wtap_rec_reset(&rec); } } CATCH(OutOfMemoryError) { @@ -842,6 +843,7 @@ cf_continue_tail(capture_file *cf, volatile int to_read, wtap_rec *rec, } to_read--; } + wtap_rec_reset(rec); } CATCH(OutOfMemoryError) { simple_message_box(ESD_TYPE_ERROR, NULL, @@ -968,6 +970,7 @@ cf_finish_tail(capture_file *cf, wtap_rec *rec, Buffer *buf, int *err) break; } read_record(cf, rec, buf, dfcode, &edt, cinfo, data_offset); + wtap_rec_reset(rec); } /* Cleanup and release all dfilter resources */ @@ -1895,6 +1898,7 @@ rescan_packets(capture_file *cf, const char *action, const char *action_item, gb on the next pass through the loop. */ prev_frame_num = fdata->num; prev_frame = fdata; + wtap_rec_reset(&rec); } epan_dissect_cleanup(&edt); @@ -2212,6 +2216,7 @@ process_specified_records(capture_file *cf, packet_range_t *range, ret = PSP_FAILED; break; } + wtap_rec_reset(&rec); } /* We're done printing the packets; destroy the progress bar if diff --git a/reordercap.c b/reordercap.c index 643732c255..c8aa70297b 100644 --- a/reordercap.c +++ b/reordercap.c @@ -122,6 +122,7 @@ frame_write(FrameRecord_t *frame, wtap *wth, wtap_dumper *pdh, wtap_file_type_subtype(wth)); exit(1); } + wtap_rec_reset(rec); } /* Comparing timestamps between 2 frames. @@ -328,6 +329,7 @@ main(int argc, char *argv[]) g_ptr_array_add(frames, newFrameRecord); prevFrame = newFrameRecord; + wtap_rec_reset(&rec); } wtap_rec_cleanup(&rec); ws_buffer_free(&buf); diff --git a/sharkd.c b/sharkd.c index eb5b4900d0..0c6dcbc525 100644 --- a/sharkd.c +++ b/sharkd.c @@ -371,6 +371,7 @@ load_cap_file(capture_file *cf, int max_packet_count, gint64 max_byte_count) while (wtap_read(cf->provider.wth, &rec, &buf, &err, &err_info, &data_offset)) { if (process_packet(cf, edt, data_offset, &rec, &buf)) { + wtap_rec_reset(&rec); /* Stop reading if we have the maximum number of packets; * When the -c option has not been used, max_packet_count * starts at 0, which practically means, never stop reading. @@ -556,6 +557,7 @@ sharkd_dissect_request(guint32 framenum, guint32 frame_ref_num, cinfo, (dissect_flags & SHARKD_DISSECT_FLAG_BYTES) ? edt.pi.data_src : NULL, data); + wtap_rec_reset(rec); epan_dissect_cleanup(&edt); return DISSECT_REQUEST_SUCCESS; } @@ -610,6 +612,7 @@ sharkd_retap(void) epan_dissect_run_with_taps(&edt, cfile.cd_t, &rec, frame_tvbuff_new_buffer(&cfile.provider, fdata, &buf), fdata, cinfo); + wtap_rec_reset(&rec); epan_dissect_reset(&edt); } @@ -687,6 +690,7 @@ sharkd_filter(const char *dftext, guint8 **result) /* if passed or ref -> frame_data_set_after_dissect */ + wtap_rec_reset(&rec); epan_dissect_reset(&edt); } diff --git a/tshark.c b/tshark.c index 7a9af3245e..22a88e48d0 100644 --- a/tshark.c +++ b/tshark.c @@ -2831,6 +2831,7 @@ capture_input_new_packets(capture_session *cap_session, int to_read) /* packet successfully read and gone through the "Read Filter" */ packet_count++; } + wtap_rec_reset(&rec); } epan_dissect_free(edt); @@ -3184,6 +3185,7 @@ process_cap_file_first_pass(capture_file *cf, int max_packet_count, break; } } + wtap_rec_reset(&rec); } if (*err != 0) status = PASS_READ_ERROR; @@ -3413,6 +3415,7 @@ process_cap_file_second_pass(capture_file *cf, wtap_dumper *pdh, } } } + wtap_rec_reset(&rec); } if (edt) @@ -3540,6 +3543,7 @@ process_cap_file_single_pass(capture_file *cf, wtap_dumper *pdh, *err = 0; /* This is not an error */ break; } + wtap_rec_reset(&rec); } if (*err != 0 && status == PASS_SUCCEEDED) { /* Error reading from the input file. */ diff --git a/ui/capture.c b/ui/capture.c index cff9d4ae6f..8d6cc030f7 100644 --- a/ui/capture.c +++ b/ui/capture.c @@ -504,6 +504,7 @@ capture_info_new_packets(int to_read, wtap *wth, info_data_t* cap_info) /*ws_warning("new packet");*/ to_read--; } + wtap_rec_reset(&rec); } } wtap_rec_cleanup(&rec); diff --git a/ui/file_dialog.c b/ui/file_dialog.c index 506f227317..91eea5ad47 100644 --- a/ui/file_dialog.c +++ b/ui/file_dialog.c @@ -83,6 +83,7 @@ get_stats_for_preview(wtap *wth, ws_file_preview_stats *stats, break; } } + wtap_rec_reset(&rec); } stats->have_times = have_times; diff --git a/wiretap/merge.c b/wiretap/merge.c index fb9638055c..cc732d664c 100644 --- a/wiretap/merge.c +++ b/wiretap/merge.c @@ -945,6 +945,7 @@ merge_process_packets(wtap_dumper *pdh, const int file_type, status = MERGE_ERR_CANT_WRITE_OUTFILE; break; } + wtap_rec_reset(rec); } if (cb) diff --git a/wiretap/wtap.c b/wiretap/wtap.c index 281dd2f192..5ec3286d4f 100644 --- a/wiretap/wtap.c +++ b/wiretap/wtap.c @@ -1669,13 +1669,20 @@ wtap_rec_init(wtap_rec *rec) */ } -/* clean up record metadata */ +/* re-initialize record */ void -wtap_rec_cleanup(wtap_rec *rec) +wtap_rec_reset(wtap_rec *rec) { wtap_block_unref(rec->block); rec->block = NULL; rec->block_was_modified = FALSE; +} + +/* clean up record metadata */ +void +wtap_rec_cleanup(wtap_rec *rec) +{ + wtap_rec_reset(rec); ws_buffer_free(&rec->options_buf); } diff --git a/wiretap/wtap.h b/wiretap/wtap.h index 8759981654..d10e6856c1 100644 --- a/wiretap/wtap.h +++ b/wiretap/wtap.h @@ -1752,6 +1752,10 @@ gboolean wtap_seek_read(wtap *wth, gint64 seek_off, wtap_rec *rec, WS_DLL_PUBLIC void wtap_rec_init(wtap_rec *rec); +/*** Re-initialize a wtap_rec structure ***/ +WS_DLL_PUBLIC +void wtap_rec_reset(wtap_rec *rec); + /*** clean up a wtap_rec structure, freeing what wtap_rec_init() allocated */ WS_DLL_PUBLIC void wtap_rec_cleanup(wtap_rec *rec);