From 9216eff1a59e9d4a24b63c16ba4c2cacd051332c Mon Sep 17 00:00:00 2001 From: dequis Date: Sun, 22 Feb 2015 03:50:48 -0300 Subject: s5bytestream: fix segfault (cleanup before trying next streamhost) This segfault happened when none of the available streamhosts can be connected to - or if at least one of them fails to connect. Before this commit, it can be reproduced reliably by setting the "proxy" setting of the account to nonsense, for example, this is what i used: proxy.example.org,1.2.3.4,7777;proxy.example.com,173.194.42.65,80 jabber_bs_recv_handshake_abort() calls jabber_bs_recv_handshake(), which is supposed to restart the handshake with the next streamhost. And it replaced bt->tf->watch_out, which held an event ID, with a newer event ID. So the replaced event ID doesn't get removed, and it gets called again when its socket is closed by the timeout - and by the time that happens, the memory is free()'d already. Boom. The patch is simple - created jabber_bs_remove_events() to cleanup those events, and use it before any code that expects to restart the cycle. So basically the same as doing b_event_remove(bt->tf->watch_out). I hope there aren't more bugs like this in this code. --- protocols/jabber/s5bytestream.c | 41 +++++++++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 14 deletions(-) diff --git a/protocols/jabber/s5bytestream.c b/protocols/jabber/s5bytestream.c index 169429d8..60d522e4 100644 --- a/protocols/jabber/s5bytestream.c +++ b/protocols/jabber/s5bytestream.c @@ -252,6 +252,31 @@ gboolean jabber_bs_abort(struct bs_transfer *bt, char *format, ...) } } +void jabber_bs_remove_events(struct bs_transfer *bt) +{ + struct jabber_transfer *tf = bt->tf; + + if (tf->watch_out) { + b_event_remove(tf->watch_out); + tf->watch_out = 0; + } + + if (tf->watch_in) { + b_event_remove(tf->watch_in); + tf->watch_in = 0; + } + + if (tf->fd != -1) { + closesocket(tf->fd); + tf->fd = -1; + } + + if (bt->connect_timeout) { + b_event_remove(bt->connect_timeout); + bt->connect_timeout = 0; + } +} + /* Bad luck */ void jabber_bs_canceled(file_transfer_t *ft, char *reason) { @@ -554,6 +579,7 @@ gboolean jabber_bs_recv_handshake_abort(struct bs_transfer *bt, char *error) shlist = g_slist_find(bt->streamhosts, bt->sh); if (shlist && shlist->next) { bt->sh = shlist->next->data; + jabber_bs_remove_events(bt); return jabber_bs_recv_handshake(bt, -1, 0); } @@ -763,20 +789,7 @@ static xt_status jabber_bs_send_handle_reply(struct im_connection *ic, struct xt } else { /* using a proxy, abort listen */ - if (tf->watch_in) { - b_event_remove(tf->watch_in); - tf->watch_in = 0; - } - - if (tf->fd != -1) { - closesocket(tf->fd); - tf->fd = -1; - } - - if (bt->connect_timeout) { - b_event_remove(bt->connect_timeout); - bt->connect_timeout = 0; - } + jabber_bs_remove_events(bt); GSList *shlist; for (shlist = jd->streamhosts; shlist; shlist = g_slist_next(shlist)) { -- cgit v1.2.3