Attempt to fix crash in pqissludp

Caused by unneeded pointer usages + not enough careful IPv6 porting

I haven't managed to reproduce the crash nor to test the fix due it
happening only when UDP relayed connection happens (apparently never on
my nodes.

I have managed to discover where the bug comes from thanks to multiple
user reports, specially to Ilario report which documented 3 crashes
happening on 0.6.4 with complete log.
This commit is contained in:
Gioacchino Mazzurco 2018-11-04 00:04:22 +01:00
parent d711e09b68
commit 7d6fba7e8c
No known key found for this signature in database
GPG Key ID: A1FBCA3872E87051
3 changed files with 82 additions and 99 deletions
libretroshare/src

@ -248,54 +248,34 @@ int pqissludp::Initiate_Connection()
}
else if (mConnectFlags & RS_CB_FLAG_MODE_UDP_RELAY)
{
std::cerr << "Calling tou_connect_via_relay(";
std::cerr << sockaddr_storage_tostring(mConnectSrcAddr) << ",";
std::cerr << sockaddr_storage_tostring(mConnectProxyAddr) << ",";
std::cerr << sockaddr_storage_tostring(remote_addr) << ")" << std::endl;
std::cerr << __PRETTY_FUNCTION__ << " Calling tou_connect_via_relay("
<< sockaddr_storage_tostring(mConnectSrcAddr) << ","
<< sockaddr_storage_tostring(mConnectProxyAddr) << ","
<< sockaddr_storage_tostring(remote_addr) << ")" << std::endl;
if(!sockaddr_storage_ipv6_to_ipv4(mConnectSrcAddr))
{
struct sockaddr_in srcaddr;
struct sockaddr_in proxyaddr;
struct sockaddr_in remoteaddr;
if(!sockaddr_storage_ipv6_to_ipv4(mConnectSrcAddr))
{
std::cerr << __PRETTY_FUNCTION__ << "Error: mConnectSrcAddr is "
<< "not valid IPv4!" << std::endl;
sockaddr_storage_dump(mConnectSrcAddr);
print_stacktrace();
return -EINVAL;
}
if(!sockaddr_storage_ipv6_to_ipv4(mConnectProxyAddr))
{
std::cerr << __PRETTY_FUNCTION__ << "Error: mConnectProxyAddr "
<< "is not valid IPv4!" << std::endl;
sockaddr_storage_dump(mConnectProxyAddr);
print_stacktrace();
return -EINVAL;
}
struct sockaddr_in *rap = (struct sockaddr_in *) &remote_addr;
struct sockaddr_in *pap = (struct sockaddr_in *) &mConnectProxyAddr;
struct sockaddr_in *sap = (struct sockaddr_in *) &mConnectSrcAddr;
srcaddr.sin_family = AF_INET;
proxyaddr.sin_family = AF_INET;
remoteaddr.sin_family = AF_INET;
srcaddr.sin_addr = sap->sin_addr;
proxyaddr.sin_addr = pap->sin_addr;
remoteaddr.sin_addr = rap->sin_addr;
srcaddr.sin_port = sap->sin_port;
proxyaddr.sin_port = pap->sin_port;
remoteaddr.sin_port = rap->sin_port;
err = tou_connect_via_relay(sockfd, &srcaddr, &proxyaddr, &remoteaddr);
std::cerr << __PRETTY_FUNCTION__ << " ERROR mConnectSrcAddr is "
<< "not a valid IPv4!" << std::endl;
sockaddr_storage_dump(mConnectSrcAddr);
print_stacktrace();
return -EINVAL;
}
if(!sockaddr_storage_ipv6_to_ipv4(mConnectProxyAddr))
{
std::cerr << __PRETTY_FUNCTION__ << " ERROR mConnectProxyAddr "
<< "is not a valid IPv4!" << std::endl;
sockaddr_storage_dump(mConnectProxyAddr);
print_stacktrace();
return -EINVAL;
}
err = tou_connect_via_relay(
sockfd,
reinterpret_cast<sockaddr_in&>(mConnectSrcAddr),
reinterpret_cast<sockaddr_in&>(mConnectProxyAddr),
reinterpret_cast<sockaddr_in&>(remote_addr) );
/*** It seems that the UDP Layer sees x 1.2 the traffic of the SSL layer.
* We need to compensate somewhere... we drop the maximum traffic to 75% of limit

@ -25,15 +25,15 @@ static const int kInitStreamTable = 5;
#include <stdlib.h>
#include <string.h>
#include "util/rstime.h"
#include <vector>
#include <iostream>
#include <errno.h>
#include "util/rstime.h"
#include "udp/udpstack.h"
#include "pqi/pqinetwork.h"
#include "tcpstream.h"
#include <vector>
#include <iostream>
#include <errno.h>
#include "util/stacktrace.h"
#define DEBUG_TOU_INTERFACE 1
#define EUSERS 87
@ -54,7 +54,7 @@ static RsMutex touMutex("touMutex");
// Mutex is used to control addition / removals from tou_streams.
// Lookup should be okay - as long as you stick to your allocated ID!
static std::vector<TcpOnUdp *> tou_streams;
static std::vector<TcpOnUdp *> tou_streams;
static int tou_inited = 0;
@ -171,19 +171,30 @@ int tou_socket(uint32_t recvIdx, uint32_t type, int /*protocol*/)
bool tou_stream_check(int sockfd)
{
if (sockfd < 0)
if(sockfd < 0)
{
std::cerr << "tou_stream_check() ERROR sockfd < 0";
std::cerr << std::endl;
std::cerr << __PRETTY_FUNCTION__ << " ERROR sockfd: " << sockfd
<< " < 0" << std::endl;
print_stacktrace();
return false;
}
if (tou_streams[sockfd] == NULL)
if(sockfd >= tou_streams.size())
{
std::cerr << "tou_stream_check() ERROR tou_streams[sockfd] == NULL";
std::cerr << std::endl;
std::cerr << __PRETTY_FUNCTION__ << " ERROR sockfd: " << sockfd
<< " out of bound!" << std::endl;
print_stacktrace();
return false;
}
if(!tou_streams[sockfd])
{
std::cerr << __PRETTY_FUNCTION__ << " ERROR tou_streams[sockfd] == NULL"
<< std::endl;
print_stacktrace();
return false;
}
return true;
}
@ -370,62 +381,55 @@ int tou_listen(int /* sockfd */ , int /* backlog */ )
*/
#define DEFAULT_RELAY_CONN_PERIOD 1
int tou_connect_via_relay(int sockfd,
const struct sockaddr_in *own_addr,
const struct sockaddr_in *proxy_addr,
const struct sockaddr_in *dest_addr)
int tou_connect_via_relay( int sockfd, const sockaddr_in& own_addr,
const sockaddr_in& proxy_addr,
const sockaddr_in& dest_addr )
{
if (!tou_stream_check(sockfd))
{
return -1;
}
TcpOnUdp *tous = tou_streams[sockfd];
std::cerr << __PRETTY_FUNCTION__ << std::endl;
if (!tou_stream_check(sockfd)) return -EINVAL;
TcpOnUdp& tous = *tou_streams[sockfd];
/* enforce that the udptype is correct */
if (tous -> udptype != TOU_RECEIVER_TYPE_UDPRELAY)
if (tous.udptype != TOU_RECEIVER_TYPE_UDPRELAY)
{
std::cerr << "tou_connect() ERROR connect method invalid for udptype";
std::cerr << std::endl;
tous -> lasterrno = EINVAL;
return -1;
std::cerr << __PRETTY_FUNCTION__ << " ERROR connect method invalid for "
<< "udptype" << std::endl;
tous.lasterrno = EINVAL;
return -EINVAL;
}
#ifdef TOU_DYNAMIC_CAST_CHECK
/* extra checking -> for testing purposes (dynamic cast) */
UdpRelayReceiver *urr = dynamic_cast<UdpRelayReceiver *>(tous->udpsr);
if (!urr)
UdpRelayReceiver* urr_ptr = dynamic_cast<UdpRelayReceiver*>(tous.udpsr);
if(!urr_ptr)
{
std::cerr << "tou_connect() ERROR cannot convert type to UdpRelayReceiver";
std::cerr << std::endl;
tous -> lasterrno = EINVAL;
return -1;
std::cerr << __PRETTY_FUNCTION__ << " ERROR cannot convert to "
<< "UdpRelayReceiver" << std::endl;
tous.lasterrno = EINVAL;
return -EINVAL;
}
#else
UdpRelayReceiver *urr = (UdpRelayReceiver *) (tous->udpsr);
#endif
UdpRelayReceiver& urr = *urr_ptr; urr_ptr = nullptr;
/* create a TCP stream to connect with. */
if (!tous->tcp)
if (!tous.tcp)
{
tous->tcp = new TcpStream(tous->udpsr);
tous.tcp = new TcpStream(tous.udpsr);
UdpRelayAddrSet addrSet(own_addr, dest_addr);
urr->addUdpPeer(tous->tcp, &addrSet, proxy_addr);
UdpRelayAddrSet addrSet(&own_addr, &dest_addr);
urr.addUdpPeer(tous.tcp, &addrSet, &proxy_addr);
}
/* We Point it at the Destination Address.
* The UdpRelayReceiver wraps and re-directs the packets to the proxy
*/
tous->tcp->connect(*dest_addr, DEFAULT_RELAY_CONN_PERIOD);
tous->tcp->tick();
if (tous->tcp->isConnected())
{
return 0;
}
tous.tcp->connect(dest_addr, DEFAULT_RELAY_CONN_PERIOD);
tous.tcp->tick();
if (tous.tcp->isConnected()) return 0;
tous -> lasterrno = EINPROGRESS;
return -1;
tous.lasterrno = EINPROGRESS;
return -EINPROGRESS;
}

@ -103,11 +103,10 @@ int tou_connect(int sockfd, const struct sockaddr *serv_addr,
socklen_t addrlen, uint32_t conn_period);
int tou_accept(int sockfd, struct sockaddr *addr, socklen_t *addrlen);
/* for relay connections */
int tou_connect_via_relay(int sockfd,
const struct sockaddr_in *own_addr,
const struct sockaddr_in *proxy_addr,
const struct sockaddr_in *dest_addr);
/// for relay connections
int tou_connect_via_relay( int sockfd, const sockaddr_in& own_addr,
const sockaddr_in& proxy_addr,
const sockaddr_in& dest_addr );
/* non-standard bonuses */
int tou_connected(int sockfd);