wiretap: fix leak of options_buf and document memory handling

Manually checked all callers of wtap_seek_read to ensure that
wtap_rec_cleanup is called. Added missing wtap_rec_cleanup to:
- Completion of sequential read: wtap_sequential_close
- Callers of wtap_seek_read:
  - users of cf_read_record_r:
    - PacketListRecord::dissect

This fixes one of the two ASAN memleak reports while running
test_tshark_z_expert_comment and test_text2pcap_sip_pcapng (the other is
about opt_comment which is still unfixed).

Vasil Velichkov also found this issue and came up with a similar fix.

Change-Id: I54a6aa70bfdb42a816d03ad4861d0ad821d0ef88
Reviewed-on: https://code.wireshark.org/review/31709
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:
Peter Wu 2019-01-24 00:15:08 +01:00 committed by Anders Broman
parent 9175a235a8
commit 9d5ab21163
4 changed files with 13 additions and 3 deletions

View File

@ -110,12 +110,12 @@ void PacketListRecord::dissect(capture_file *cap_file, bool dissect_color)
return;
}
memset(&rec, 0, sizeof rec);
if (dissect_columns) {
cinfo = &cap_file->cinfo;
}
wtap_rec_init(&rec);
ws_buffer_init(&buf, 1500);
if (!cf_read_record_r(cap_file, fdata_, &rec, &buf)) {
/*
@ -138,6 +138,7 @@ void PacketListRecord::dissect(capture_file *cap_file, bool dissect_color)
colorized_ = true;
}
ws_buffer_free(&buf);
wtap_rec_cleanup(&rec);
return; /* error reading the record */
}
@ -193,6 +194,7 @@ void PacketListRecord::dissect(capture_file *cap_file, bool dissect_color)
epan_dissect_cleanup(&edt);
ws_buffer_free(&buf);
wtap_rec_cleanup(&rec);
}
// This assumes only one packet list. We might want to move this to

View File

@ -1128,7 +1128,9 @@ fail:
return NULL;
success:
wth->rec_data = (struct Buffer *)g_malloc(sizeof(struct Buffer));
/* wth->rec_data is used for the sequential read and will be freed by
* wtap_sequential_close (which is also called by wtap_close). */
wth->rec_data = g_new(struct Buffer, 1);
ws_buffer_init(wth->rec_data, 1500);
if ((wth->file_type_subtype == WTAP_FILE_TYPE_SUBTYPE_PCAP) ||

View File

@ -1341,7 +1341,11 @@ pcapng_read_packet_block(FILE_T fh, pcapng_block_header_t *bh, pcapng_t *pn, wta
block_read - /* fixed and variable part, including padding */
(int)sizeof(bh->block_total_length);
/* Allocate enough memory to hold all options */
/* Ensure sufficient temporary memory to hold all options. It is not freed
* on return to avoid frequent reallocations. When called for sequential
* read (wtap_read), "wblock->rec == &wth->rec" (options_buf will be freed
* by wtap_sequential_close). For random access, memory is managed by the
* caller of wtap_seek_read. */
opt_cont_buf_len = to_read;
ws_buffer_assure_space(&wblock->rec->options_buf, opt_cont_buf_len);
opt_ptr = ws_buffer_start_ptr(&wblock->rec->options_buf);

View File

@ -1195,6 +1195,8 @@ wtap_sequential_close(wtap *wth)
wth->fh = NULL;
}
wtap_rec_cleanup(&wth->rec);
if (wth->rec_data) {
ws_buffer_free(wth->rec_data);
g_free(wth->rec_data);