Skip to content

Commit

Permalink
[#17478] free blocks in more places
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Boolean263 authored and guyharris committed Aug 10, 2021
1 parent 4aee405 commit 6e12643
Show file tree
Hide file tree
Showing 12 changed files with 36 additions and 2 deletions.
1 change: 1 addition & 0 deletions capinfos.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions debian/libwiretap0.symbols
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 3 additions & 0 deletions editcap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -2285,6 +2287,7 @@ main(int argc, char *argv[])
wtap_dump_params_cleanup(&params);
if (wth != NULL)
wtap_close(wth);
wtap_rec_reset(&read_rec);
wtap_cleanup();
free_progdirs();
if (capture_comments != NULL) {
Expand Down
5 changes: 5 additions & 0 deletions file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 */
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions reordercap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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);
Expand Down
4 changes: 4 additions & 0 deletions sharkd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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;
}
Expand Down Expand Up @@ -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);
}

Expand Down Expand Up @@ -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);
}

Expand Down
4 changes: 4 additions & 0 deletions tshark.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -3413,6 +3415,7 @@ process_cap_file_second_pass(capture_file *cf, wtap_dumper *pdh,
}
}
}
wtap_rec_reset(&rec);
}

if (edt)
Expand Down Expand Up @@ -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. */
Expand Down
1 change: 1 addition & 0 deletions ui/capture.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions ui/file_dialog.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 1 addition & 0 deletions wiretap/merge.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
11 changes: 9 additions & 2 deletions wiretap/wtap.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}

Expand Down
4 changes: 4 additions & 0 deletions wiretap/wtap.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down

0 comments on commit 6e12643

Please sign in to comment.