Changeset 9216eff

Timestamp:

2015-02-22T18:53:21Z (10 years ago)

Author:

dequis <dx@…>

Branches:

master

Children:

5ff4618

Parents:

da6f167

git-author:

dequis <dx@…> (22-02-15 06:50:48)

git-committer:

dequis <dx@…> (22-02-15 18:53:21)

Message:

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.

File:

• protocols/jabber/s5bytestream.c (modified) (3 diffs)

protocols/jabber/s5bytestream.c

@@ -253 +253 @@
 }
 
+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)
@@ -555 +580 @@
         if (shlist && shlist->next) {
                 bt->sh = shlist->next->data;
+                jabber_bs_remove_events(bt);
                 return jabber_bs_recv_handshake(bt, -1, 0);
         }
@@ -764 +790 @@
                 /* 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;