Fix crashes when trying unreachable nodes.
authorEtienne Dechamps <etienne@edechamps.fr>
Sat, 23 May 2015 09:24:00 +0000 (10:24 +0100)
committerEtienne Dechamps <etienne@edechamps.fr>
Sat, 23 May 2015 09:24:00 +0000 (10:24 +0100)
timeout_handler() calls try_tx(c->node) when c->edge exists.
Unfortunately, the existence of c->edge is not enough to conclude that
the node is reachable.

In fact, during connection establishment, there is a short period of
time where we create an edge for the node at the other end of the
metaconnection, but we don't have one from the other side yet.
Unfortunately, if timeout_handler() runs during that short time
window, it will call try_tx() on an unreachable node, which makes
things explode because that function is not prepared to handle that
case.

A typical symptom of this race condition is a hard SEGFAULT while trying
to send packets using metaconnections that don't exist, due to
n->nexthop containing garbage.

This patch fixes the issue by making try_tx() check for reachability,
and then making all code paths use try_tx() instead of the more
specialized methods so that they go through the check.

This regression was introduced in
eb7a0db18ea71a44999d6a37b4b179dac0ed9bc7.

src/net_packet.c

index fc24c9a545a6ba72bd5ec0fe505b55580e368ea9..e4b839ef33ec5f8352a4dfc23cc08d269eedf142 100644 (file)
@@ -1195,7 +1195,7 @@ static void try_tx_sptps(node_t *n, bool mtu) {
        if(via != n) {
                if((via->options >> 24) < 4)
                        return;
-               return try_tx_sptps(via, mtu);
+               return try_tx(via, mtu);
        }
 
        /* Otherwise, try to establish UDP connectivity. */
@@ -1208,7 +1208,7 @@ static void try_tx_sptps(node_t *n, bool mtu) {
           while we try to establish direct connectivity. */
 
        if(!n->status.udp_confirmed && n != n->nexthop && (n->nexthop->options >> 24) >= 4)
-               try_tx_sptps(n->nexthop, mtu);
+               try_tx(n->nexthop, mtu);
 }
 
 static void try_tx_legacy(node_t *n, bool mtu) {
@@ -1233,6 +1233,8 @@ static void try_tx_legacy(node_t *n, bool mtu) {
 }
 
 void try_tx(node_t *n, bool mtu) {
+       if(!n->status.reachable)
+               return;
        if(n->status.sptps)
                try_tx_sptps(n, mtu);
        else
@@ -1269,7 +1271,7 @@ void send_packet(node_t *n, vpn_packet_t *packet) {
 
        if(n->status.sptps) {
                send_sptps_packet(n, packet);
-               try_tx_sptps(n, true);
+               try_tx(n, true);
                return;
        }
 
@@ -1289,7 +1291,7 @@ void send_packet(node_t *n, vpn_packet_t *packet) {
        }
 
        send_udppacket(via, packet);
-       try_tx_legacy(via, true);
+       try_tx(via, true);
 }
 
 void broadcast_packet(const node_t *from, vpn_packet_t *packet) {
@@ -1472,7 +1474,7 @@ skip_harder:
 
                if(to != myself) {
                        send_sptps_data(to, from, 0, DATA(&pkt), pkt.len);
-                       try_tx_sptps(to, true);
+                       try_tx(to, true);
                        return;
                }
        } else {