Skip to content

Commit

Permalink
s5bytestream: fix segfault (cleanup before trying next streamhost)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
dequis committed Feb 22, 2015
1 parent da6f167 commit 9216eff
Showing 1 changed file with 27 additions and 14 deletions.
41 changes: 27 additions & 14 deletions protocols/jabber/s5bytestream.c
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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);
}

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

0 comments on commit 9216eff

Please sign in to comment.