osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

Qt: Fix memory leak in TCP Stream Dialog 

Do not copy addresses at when dialog opens, they will be initialized
in tapall_tcpip_packet(). Do not clear addresses when switching stream,
they will be properly removed in graph_segment_list_free().

Correctly free addresses in graph_segment_list_free() which is called
when switching stream and when closing the dialog. Free copied addresses
when switching direction (address swap).

Remove redundant and unused code.

Change-Id: I4328aa4df333f59c587f841b74a24dc71d329079
Reviewed-on: https://code.wireshark.org/review/36840
Petri-Dish: Stig Bjørlykke <stig@bjorlykke.org>
Tested-by: Petri Dish Buildbot
Reviewed-by: Tomasz Moń <desowin@gmail.com>
This commit is contained in:
Stig Bjørlykke 2020-04-13 23:00:48 +02:00 committed by Tomasz Moń
parent d3c4dfa9ee
commit 6b589151a2
3 changed files with 37 additions and 74 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
ui/qt/tcp_stream_dialog.cpp
View File

@ -123,23 +123,14 @@ TCPStreamDialog::TCPStreamDialog(QWidget *parent, capture_file *cf, tcp_graph_ty
num_sack_ranges_(-1),
ma_window_size_(1.0)
{
struct segment current;
int graph_idx = -1;
ui->setupUi(this);
if (parent) loadGeometry(parent->width() * 2 / 3, parent->height() * 4 / 5);
setAttribute(Qt::WA_DeleteOnClose, true);
graph_.type = GRAPH_UNDEFINED;
set_address(&graph_.src_address, AT_NONE, 0, NULL);
graph_.src_port = 0;
set_address(&graph_.dst_address, AT_NONE, 0, NULL);
graph_.dst_port = 0;
graph_.stream = 0;
graph_.segments = NULL;
struct tcpheader *header = select_tcpip_session(cap_file_, &current);
if (!header) {
guint32 th_stream = select_tcpip_session(cap_file_);
if (th_stream == G_MAXUINT32) {
done(QDialog::Rejected);
return;
}
@ -196,11 +187,7 @@ TCPStreamDialog::TCPStreamDialog(QWidget *parent, capture_file *cf, tcp_graph_ty
memset (&graph_, 0, sizeof(graph_));
graph_.type = graph_type;
copy_address(&graph_.src_address, &current.ip_src);
graph_.src_port = current.th_sport;
copy_address(&graph_.dst_address, &current.ip_dst);
graph_.dst_port = current.th_dport;
graph_.stream = header->th_stream;
graph_.stream = th_stream;
findStream();
showWidgetsForGraphType();
@ -369,6 +356,8 @@ TCPStreamDialog::TCPStreamDialog(QWidget *parent, capture_file *cf, tcp_graph_ty
TCPStreamDialog::~TCPStreamDialog()
{
graph_segment_list_free(&graph_);
delete ui;
}
@ -518,7 +507,7 @@ void TCPStreamDialog::findStream()
ui->streamNumberSpinBox->clearFocus();
ui->streamNumberSpinBox->setEnabled(false);
graph_segment_list_free(&graph_);
graph_segment_list_get(cap_file_, &graph_, TRUE);
graph_segment_list_get(cap_file_, &graph_);
ui->streamNumberSpinBox->setEnabled(true);
if (spin_box_focused)
ui->streamNumberSpinBox->setFocus();
@ -2102,10 +2091,13 @@ void TCPStreamDialog::on_actionSwitchDirection_triggered()
copy_address(&tmp_addr, &graph_.src_address);
tmp_port = graph_.src_port;
free_address(&graph_.src_address);
copy_address(&graph_.src_address, &graph_.dst_address);
graph_.src_port = graph_.dst_port;
free_address(&graph_.dst_address);
copy_address(&graph_.dst_address, &tmp_addr);
graph_.dst_port = tmp_port;
free_address(&tmp_addr);
fillGraph(/*reset_axes=*/true, /*set_focus=*/false);
}
@ -2195,8 +2187,6 @@ void TCPStreamDialog::GraphUpdater::doUpdate()
if ((int(dialog_->graph_.stream) != new_stream) &&
(new_stream >= 0 && new_stream < int(get_tcp_stream_count()))) {
dialog_->graph_.stream = new_stream;
clear_address(&dialog_->graph_.src_address);
clear_address(&dialog_->graph_.dst_address);
dialog_->findStream();
}
dialog_->fillGraph(reset_axes, /*set_focus =*/false);

76
ui/tap-tcp-stream.c
View File

@ -29,7 +29,6 @@
#include "tap-tcp-stream.h"
typedef struct _tcp_scan_t {
struct segment *current;
int direction;
struct tcp_graph *tg;
struct segment *last;
@ -101,9 +100,8 @@ tapall_tcpip_packet(void *pct, packet_info *pinfo, epan_dissect_t *edt _U_, cons
/* here we collect all the external data we will ever need */
void
graph_segment_list_get(capture_file *cf, struct tcp_graph *tg, gboolean stream_known)
graph_segment_list_get(capture_file *cf, struct tcp_graph *tg)
{
struct segment current;
GString *error_string;
tcp_scan_t ts;
@ -113,30 +111,11 @@ graph_segment_list_get(capture_file *cf, struct tcp_graph *tg, gboolean stream_k
return;
}
if (!stream_known) {
struct tcpheader *header = select_tcpip_session(cf, &current);
if (!header) return;
if (tg->type == GRAPH_THROUGHPUT) {
ts.direction = COMPARE_CURR_DIR;
} else {
ts.direction = COMPARE_ANY_DIR;
}
/* Remember stream info in graph */
copy_address(&tg->src_address, &current.ip_src);
tg->src_port = current.th_sport;
copy_address(&tg->dst_address, &current.ip_dst);
tg->dst_port = current.th_dport;
tg->stream = header->th_stream;
} else {
ts.direction = COMPARE_ANY_DIR;
}
/* rescan all the packets and pick up all interesting tcp headers.
* we only filter for TCP here for speed and do the actual compare
* in the tap listener
*/
ts.current = &current;
ts.direction = COMPARE_ANY_DIR;
ts.tg = tg;
ts.last = NULL;
error_string = register_tap_listener("tcp", &ts, "tcp", 0, NULL, tapall_tcpip_packet, NULL, NULL);
@ -155,8 +134,13 @@ graph_segment_list_free(struct tcp_graph *tg)
{
struct segment *segment;
free_address(&tg->src_address);
free_address(&tg->dst_address);
while (tg->segments) {
segment = tg->segments->next;
free_address(&tg->segments->ip_src);
free_address(&tg->segments->ip_dst);
g_free(tg->segments);
tg->segments = segment;
}
@ -267,31 +251,31 @@ tap_tcpip_packet(void *pct, packet_info *pinfo _U_, epan_dissect_t *edt _U_, con
* then present the user with a dialog where the user can select WHICH tcp
* session to graph.
*/
struct tcpheader *
select_tcpip_session(capture_file *cf, struct segment *hdrs)
guint32
select_tcpip_session(capture_file *cf)
{
frame_data *fdata;
epan_dissect_t edt;
dfilter_t *sfcode;
guint32 th_stream;
gchar *err_msg;
GString *error_string;
nstime_t rel_ts;
th_t th = {0, {NULL, NULL, NULL, NULL, NULL, NULL, NULL, NULL}};
if (!cf || !hdrs) {
return NULL;
if (!cf) {
return G_MAXUINT32;
}
/* no real filter yet */
if (!dfilter_compile("tcp", &sfcode, &err_msg)) {
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK, "%s", err_msg);
g_free(err_msg);
return NULL;
return G_MAXUINT32;
}
/* dissect the current record */
if (!cf_read_current_record(cf)) {
return NULL; /* error reading the record */
return G_MAXUINT32; /* error reading the record */
}
fdata = cf->current_frame;
@ -309,9 +293,9 @@ select_tcpip_session(capture_file *cf, struct segment *hdrs)
epan_dissect_run_with_taps(&edt, cf->cd_t, &cf->rec,
frame_tvbuff_new_buffer(&cf->provider, fdata, &cf->buf),
fdata, NULL);
rel_ts = edt.pi.rel_ts;
epan_dissect_cleanup(&edt);
remove_tap_listener(&th);
dfilter_free(sfcode);
if (th.num_hdrs == 0) {
/* This "shouldn't happen", as our menu items shouldn't
@ -320,7 +304,7 @@ select_tcpip_session(capture_file *cf, struct segment *hdrs)
* to determine whether to enable any of our menu items. */
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
"Selected packet isn't a TCP segment or is truncated");
return NULL;
return G_MAXUINT32;
}
/* XXX fix this later, we should show a dialog allowing the user
to select which session he wants here
@ -330,27 +314,19 @@ select_tcpip_session(capture_file *cf, struct segment *hdrs)
simple_dialog(ESD_TYPE_ERROR, ESD_BTN_OK,
"The selected packet has more than one TCP unique conversation "
"in it.");
return NULL;
return G_MAXUINT32;
}
/* For now, still always choose the first/only one */
hdrs->num = fdata->num;
hdrs->rel_secs = (guint32) rel_ts.secs;
hdrs->rel_usecs = rel_ts.nsecs/1000;
/* Currently unused
hdrs->abs_secs = (guint32) fdata->abs_ts.secs;
hdrs->abs_usecs = fdata->abs_ts.nsecs/1000;
*/
hdrs->th_seq = th.tcphdrs[0]->th_seq;
hdrs->th_ack = th.tcphdrs[0]->th_ack;
hdrs->th_win = th.tcphdrs[0]->th_win;
hdrs->th_flags = th.tcphdrs[0]->th_flags;
hdrs->th_sport = th.tcphdrs[0]->th_sport;
hdrs->th_dport = th.tcphdrs[0]->th_dport;
hdrs->th_seglen = th.tcphdrs[0]->th_seglen;
copy_address(&hdrs->ip_src, &th.tcphdrs[0]->ip_src);
copy_address(&hdrs->ip_dst, &th.tcphdrs[0]->ip_dst);
return th.tcphdrs[0];
th_stream = th.tcphdrs[0]->th_stream;
for (int n = 0; n < th.num_hdrs; n++) {
free_address(&th.tcphdrs[n]->ip_src);
free_address(&th.tcphdrs[n]->ip_dst);
g_free(th.tcphdrs[n]);
}
return th_stream;
}
int rtt_is_retrans(struct rtt_unack *list, unsigned int seqno)

7
ui/tap-tcp-stream.h
View File

@ -71,11 +71,8 @@ struct tcp_graph {
* destination address types are AT_NONE the address and port
* information will be filled in using the first packet in the
* specified stream.
* @param stream_known If FALSE, session information will be filled in using
* the currently selected packet. If FALSE, session information will
* be matched against tg.
*/
void graph_segment_list_get(capture_file *cf, struct tcp_graph *tg, gboolean stream_known );
void graph_segment_list_get(capture_file *cf, struct tcp_graph *tg);
void graph_segment_list_free(struct tcp_graph * );
/* for compare_headers() */
@ -88,7 +85,7 @@ int compare_headers(address *saddr1, address *daddr1, guint16 sport1, guint16 dp
int get_num_dsegs(struct tcp_graph * );
int get_num_acks(struct tcp_graph *, int * );
struct tcpheader *select_tcpip_session(capture_file *, struct segment * );
guint32 select_tcpip_session(capture_file *);
/* This is used by rtt module only */
struct rtt_unack {