Merge pull request #1392 from G10h4ck/hotfix_udp_crash

WIP: Attempt to fix crash in pqissludp

libretroshare/src/pqi/pqissludp.cc

@@ -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 << __PRETTY_FUNCTION__ << " Calling tou_connect_via_relay("
-		std::cerr << sockaddr_storage_tostring(mConnectSrcAddr) << ",";
+		          << sockaddr_storage_tostring(mConnectSrcAddr) << ","
-		std::cerr << sockaddr_storage_tostring(mConnectProxyAddr) << ",";
+		          << sockaddr_storage_tostring(mConnectProxyAddr) << ","
-		std::cerr << sockaddr_storage_tostring(remote_addr) << ")" << std::endl;
+		          << sockaddr_storage_tostring(remote_addr) << ")" << std::endl;
-
-		
-		{
-			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 "
+			std::cerr << __PRETTY_FUNCTION__ << " ERROR mConnectSrcAddr is "
-				          << "not valid IPv4!" << std::endl;
+			          << "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 "
+			std::cerr << __PRETTY_FUNCTION__ << " ERROR mConnectProxyAddr "
-				          << "is not valid IPv4!" << std::endl;
+			          << "is not a valid IPv4!" << std::endl;
 			sockaddr_storage_dump(mConnectProxyAddr);
 			print_stacktrace();
 			return -EINVAL;
 
 		}
 
-			struct sockaddr_in *rap = (struct sockaddr_in *) &remote_addr;
+		err = tou_connect_via_relay(
-			struct sockaddr_in *pap = (struct sockaddr_in *) &mConnectProxyAddr;
+		            sockfd,
-			struct sockaddr_in *sap = (struct sockaddr_in *) &mConnectSrcAddr;
+		            reinterpret_cast<sockaddr_in&>(mConnectSrcAddr),
-			
+		            reinterpret_cast<sockaddr_in&>(mConnectProxyAddr),
-			srcaddr.sin_family = AF_INET;
+		            reinterpret_cast<sockaddr_in&>(remote_addr) );
-			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);
-			
-		}
 
 /*** 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

libretroshare/src/tcponudp/bss_tou.c

@@ -99,28 +99,13 @@ static int  BIO_get_init(BIO *a) { return a->init; }
 static int  BIO_get_shutdown(BIO *a) { return a->shutdown; }
 static void BIO_set_init(BIO *a,int i) { a->init=i; }
 static void BIO_set_data(BIO *a,void *p) { a->ptr = p; }
+long (*BIO_meth_get_ctrl(const BIO_METHOD* biom)) (BIO*, int, long, void*)
+{ return biom->ctrl; }
 
 #endif
 
-#else
+static BIO_METHOD methods_tou_sockp =
-
+{
-typedef struct bio_method_st {
-    int type;
-    const char *name;
-    int (*bwrite) (BIO *, const char *, int);
-    int (*bread) (BIO *, char *, int);
-    int (*bputs) (BIO *, const char *);
-    int (*bgets) (BIO *, char *, int);
-    long (*ctrl) (BIO *, int, long, void *);
-    int (*create) (BIO *);
-    int (*destroy) (BIO *);
-    long (*callback_ctrl) (BIO *, int, bio_info_cb *);
-} BIO_METHOD;
-
-#endif
-
-static BIO_METHOD methods_tou_sockp=
-	{
     BIO_TYPE_TOU_SOCKET,
     "tou_socket",
     tou_socket_write,
@@ -131,16 +116,38 @@ static BIO_METHOD methods_tou_sockp=
     tou_socket_new,
     tou_socket_free,
     NULL,
-	};
+};
 
-BIO_METHOD *BIO_s_tou_socket(void)
+BIO_METHOD* BIO_s_tou_socket(void)
-	{
+{
 #ifdef DEBUG_TOU_BIO
 	fprintf(stderr, "BIO_s_tou_socket(void)\n");
 #endif
 	return(&methods_tou_sockp);
+}
+
+#else
+
+BIO_METHOD* BIO_s_tou_socket(void)
+{
+	static BIO_METHOD* methods_tou_sockp_ptr = NULL;
+	if(!methods_tou_sockp_ptr)
+	{
+		 methods_tou_sockp_ptr = BIO_meth_new(BIO_TYPE_TOU_SOCKET, "tou_socket");
+
+		BIO_meth_set_write(   methods_tou_sockp_ptr, tou_socket_write );
+		BIO_meth_set_read(    methods_tou_sockp_ptr, tou_socket_read  );
+		BIO_meth_set_puts(    methods_tou_sockp_ptr, tou_socket_puts  );
+		BIO_meth_set_ctrl(    methods_tou_sockp_ptr, tou_socket_ctrl  );
+		BIO_meth_set_create(  methods_tou_sockp_ptr, tou_socket_new   );
+		BIO_meth_set_destroy( methods_tou_sockp_ptr, tou_socket_free  );
 	}
 
+	return methods_tou_sockp_ptr;
+}
+
+#endif
+
 BIO *BIO_new_tou_socket(int fd, int close_flag)
 	{
 	BIO *ret;
@@ -260,17 +267,16 @@ static long tou_socket_ctrl(BIO *b, int cmd, long num, void *ptr)
 		break;
 	case BIO_C_SET_FD:
 		tou_socket_free(b);
-		ret = BIO_s_fd()->ctrl(b,cmd,num,ptr) ;
+		ret = BIO_meth_get_ctrl((BIO_METHOD*)BIO_s_fd())(b,cmd,num,ptr);
-
 		break;
 	case BIO_C_GET_FD:
-		ret = BIO_s_fd()->ctrl(b,cmd,num,ptr) ;
+		ret = BIO_meth_get_ctrl((BIO_METHOD*)BIO_s_fd())(b,cmd,num,ptr);
 		break;
 	case BIO_CTRL_GET_CLOSE:
-		ret = BIO_s_fd()->ctrl(b,cmd,num,ptr) ;
+		ret = BIO_meth_get_ctrl((BIO_METHOD*)BIO_s_fd())(b,cmd,num,ptr);
 		break;
 	case BIO_CTRL_SET_CLOSE:
-		ret = BIO_s_fd()->ctrl(b,cmd,num,ptr) ;
+		ret = BIO_meth_get_ctrl((BIO_METHOD*)BIO_s_fd())(b,cmd,num,ptr);
 		break;
 	case BIO_CTRL_PENDING:
 		ret = tou_maxread(BIO_get_fd(b,NULL));

libretroshare/src/tcponudp/tou.cc

@@ -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 "util/stacktrace.h"
-#include <iostream>
-
-#include <errno.h>
 
 #define DEBUG_TOU_INTERFACE 1
 #define EUSERS          87
@@ -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 << __PRETTY_FUNCTION__ << " ERROR sockfd: " << sockfd
-		std::cerr << std::endl;
+		          << " < 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 << __PRETTY_FUNCTION__ << " ERROR sockfd: " << sockfd
-		std::cerr << std::endl;
+		          << " 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, 
+int tou_connect_via_relay( int sockfd, const sockaddr_in& own_addr,
-			const struct sockaddr_in *own_addr, 
+                           const sockaddr_in& proxy_addr,
-			const struct sockaddr_in *proxy_addr, 
+                           const sockaddr_in& dest_addr )
-			const struct sockaddr_in *dest_addr)
 
 {
-	if (!tou_stream_check(sockfd))
+	std::cerr << __PRETTY_FUNCTION__ << std::endl;
-	{
+
-		return -1;
+	if (!tou_stream_check(sockfd)) return -EINVAL;
-	}
+
-	TcpOnUdp *tous = tou_streams[sockfd];
+	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 << __PRETTY_FUNCTION__ << " ERROR connect method invalid for "
-		std::cerr << std::endl;
+		          << "udptype" << std::endl;
-		tous -> lasterrno = EINVAL;
+		tous.lasterrno = EINVAL;
-		return -1;
+		return -EINVAL;
 	}
 
-#ifdef TOU_DYNAMIC_CAST_CHECK 
+	UdpRelayReceiver* urr_ptr = dynamic_cast<UdpRelayReceiver*>(tous.udpsr);
-	/* extra checking -> for testing purposes (dynamic cast) */
+	if(!urr_ptr)
-	UdpRelayReceiver *urr = dynamic_cast<UdpRelayReceiver *>(tous->udpsr);
-	if (!urr)
 	{
-		std::cerr << "tou_connect() ERROR cannot convert type to UdpRelayReceiver";
+		std::cerr << __PRETTY_FUNCTION__ << " ERROR cannot convert to "
-		std::cerr << std::endl;
+		          << "UdpRelayReceiver" << std::endl;
-		tous -> lasterrno = EINVAL;
+		tous.lasterrno = EINVAL;
-		return -1;
+		return -EINVAL;
 	}
-#else
+
-	UdpRelayReceiver *urr = (UdpRelayReceiver *) (tous->udpsr);
+	UdpRelayReceiver& urr = *urr_ptr; urr_ptr = nullptr;
-#endif
 
 	/* 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);
+		UdpRelayAddrSet addrSet(&own_addr, &dest_addr);
-		urr->addUdpPeer(tous->tcp, &addrSet, proxy_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->connect(dest_addr, DEFAULT_RELAY_CONN_PERIOD);
-	tous->tcp->tick();
+	tous.tcp->tick();
-	if (tous->tcp->isConnected())
+	if (tous.tcp->isConnected()) return 0;
-	{
-		return 0;	
-	}
 
-	tous -> lasterrno = EINPROGRESS;
+	tous.lasterrno = EINPROGRESS;
-	return -1;
+	return -EINPROGRESS;
 }
 
 

libretroshare/src/tcponudp/tou.h

@@ -103,12 +103,6 @@ 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);
-
 /* non-standard bonuses */
 int	tou_connected(int sockfd);
 int 	tou_listenfor(int sockfd, const struct sockaddr *serv_addr, socklen_t addrlen);
@@ -131,6 +125,14 @@ int	tou_maxwrite(int sockfd);
 
 #ifdef  __cplusplus
 }
+
+typedef struct sockaddr_in sockaddr_in;
+
+/// 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 );
+
 #endif
 #endif
 

libretroshare/src/util/stacktrace.h

@@ -1,9 +1,7 @@
 /*******************************************************************************
- * libretroshare/src/util: smallobject.h                                       *
+ * libretroshare                                                               *
  *                                                                             *
- * libretroshare: retroshare core library                                      *
+ * Copyright (C) 2016-2018 Gioacchino Mazzurco <gio@eigenlab.org>              *
- *                                                                             *
- * Copyright (C) 2016 Gioacchino Mazzurco <gio@eigenlab.org>                   *
  * Copyright (C) 2008 Timo Bingmann http://idlebox.net/                        *
  *                                                                             *
  * This program is free software: you can redistribute it and/or modify        *
@@ -20,24 +18,39 @@
  * along with this program. If not, see <https://www.gnu.org/licenses/>.       *
  *                                                                             *
  *******************************************************************************/
-#ifndef _STACKTRACE_H_
+#pragma once
-#define _STACKTRACE_H_
 
-#include <stdio.h>
+#include <cstdio>
+#include <csignal>
+#include <cstdlib>
 
 #if defined(__linux__) && defined(__GLIBC__)
 
-#include <stdlib.h>
 #include <execinfo.h>
 #include <cxxabi.h>
 
-/** Print a demangled stack backtrace of the caller function to FILE* out. */
+/**
-static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames = 63)
+ * @brief Print a backtrace to FILE* out.
+ * @param[in] demangle true to demangle C++ symbols requires malloc working, in
+ *	some patological cases like a SIGSEGV received during a malloc this would
+ *	cause deadlock so pass false if you may be in such situation (like in a
+ *	SIGSEGV handler )
+ * @param[in] out output file
+ * @param[in] maxFrames maximum number of stack frames you want to bu printed
+ */
+static inline void print_stacktrace(
+        bool demangle = true, FILE *out = stderr, unsigned int maxFrames = 63 )
 {
+	if(!out)
+	{
+		fprintf(stderr, "print_stacktrace invalid output file!\n");
+		return;
+	}
+
 	fprintf(out, "stack trace:\n");
 
 	// storage array for stack trace address data
-	void* addrlist[max_frames+1];
+	void* addrlist[maxFrames+1];
 
 	// retrieve current stack addresses
 	int addrlen = backtrace(addrlist, sizeof(addrlist) / sizeof(void*));
@@ -48,6 +61,19 @@ static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames
 		return;
 	}
 
+	if(!demangle)
+	{
+		int outFd = fileno(out);
+		if(outFd < 0)
+		{
+			fprintf(stderr, "print_stacktrace invalid output file descriptor!\n");
+			return;
+		}
+
+		backtrace_symbols_fd(addrlist, addrlen, outFd);
+		return;
+	}
+
 	// resolve addresses into strings containing "filename(function+address)",
 	// this array must be free()-ed
 	char** symbollist = backtrace_symbols(addrlist, addrlen);
@@ -62,8 +88,8 @@ static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames
 	{
 		char *begin_name = 0, *begin_offset = 0, *end_offset = 0;
 
-		// find parentheses and +address offset surrounding the mangled name:
+		/* find parentheses and +address offset surrounding the mangled
-		// ./module(function+0x15c) [0x8048a6d]
+		 * name: ./module(function+0x15c) [0x8048a6d] */
 		for (char *p = symbollist[i]; *p; ++p)
 		{
 			if (*p == '(') begin_name = p;
@@ -75,7 +101,8 @@ static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames
 			}
 		}
 
-		if (begin_name && begin_offset && end_offset && begin_name < begin_offset)
+		if ( begin_name && begin_offset && end_offset
+		     && begin_name < begin_offset )
 		{
 			*begin_name++ = '\0';
 			*begin_offset++ = '\0';
@@ -86,17 +113,20 @@ static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames
 			// __cxa_demangle():
 
 			int status;
-			char* ret = abi::__cxa_demangle(begin_name, funcname, &funcnamesize, &status);
+			char* ret = abi::__cxa_demangle(
+			            begin_name, funcname, &funcnamesize, &status );
 			if (status == 0)
 			{
 				funcname = ret; // use possibly realloc()-ed string
-				fprintf(out, "  %s : %s+%s\n", symbollist[i], funcname, begin_offset);
+				fprintf( out, "  %s : %s+%s\n",
+				         symbollist[i], funcname, begin_offset );
 			}
 			else
 			{
 				// demangling failed. Output function name as a C function with
 				// no arguments.
-				fprintf(out, "  %s : %s()+%s\n", symbollist[i], begin_name, begin_offset);
+				fprintf( out, "  %s : %s()+%s\n",
+				         symbollist[i], begin_name, begin_offset );
 			}
 		}
 		else
@@ -111,12 +141,61 @@ static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames
 }
 
 #else // defined(__linux__) && defined(__GLIBC__)
-static inline void print_stacktrace(FILE *out = stderr, unsigned int max_frames = 63)
+static inline void print_stacktrace(
+        bool demangle = true, FILE *out = stderr, unsigned int max_frames = 63 )
 {
+	(void) demangle;
 	(void) max_frames;
 
 	fprintf(out, "TODO: 2016/01/01 print_stacktrace not implemented yet for WINDOWS_SYS and ANDROID\n");
 }
 #endif // defined(__linux__) && defined(__GLIBC__)
 
-#endif // _STACKTRACE_H_
+/**
+ * @brief CrashStackTrace catch crash signals and print stack trace
+ * Inspired too https://oroboro.com/stack-trace-on-crash/
+ */
+struct CrashStackTrace
+{
+	CrashStackTrace()
+	{
+		signal(SIGABRT, &CrashStackTrace::abortHandler);
+		signal(SIGSEGV, &CrashStackTrace::abortHandler);
+		signal(SIGILL,  &CrashStackTrace::abortHandler);
+		signal(SIGFPE,  &CrashStackTrace::abortHandler);
+#ifdef SIGBUS
+		signal(SIGBUS,  &CrashStackTrace::abortHandler);
+#endif
+	}
+
+	static void abortHandler(int signum)
+	{
+		// associate each signal with a signal name string.
+		const char* name = nullptr;
+		switch(signum)
+		{
+		case SIGABRT: name = "SIGABRT";  break;
+		case SIGSEGV: name = "SIGSEGV";  break;
+		case SIGILL:  name = "SIGILL";   break;
+		case SIGFPE:  name = "SIGFPE";   break;
+#ifdef SIGBUS
+		case SIGBUS:  name = "SIGBUS";   break;
+#endif
+		}
+
+		/** Notify the user which signal was caught. We use printf, because this
+		 * is the most basic output function. Once you get a crash, it is
+		 * possible that more complex output systems like streams and the like
+		 * may be corrupted. So we make the most basic call possible to the
+		 * lowest level, most standard print function. */
+		if(name)
+			fprintf(stderr, "Caught signal %d (%s)\n", signum, name);
+		else
+			fprintf(stderr, "Caught signal %d\n", signum);
+
+		print_stacktrace(false);
+
+		exit(-signum);
+	}
+};
+

retroshare-gui/src/main.cpp

@@ -19,6 +19,10 @@
  *  Boston, MA  02110-1301, USA.
  ****************************************************************/
 
+#include "util/stacktrace.h"
+
+CrashStackTrace gCrashStackTrace;
+
 #include <QObject>
 #include <QMessageBox>
 #include <QSplashScreen>

retroshare-service/src/retroshare-service.cc

@@ -16,6 +16,10 @@
  * along with this program.  If not, see <http://www.gnu.org/licenses/>.
  */
 
+#include "util/stacktrace.h"
+
+CrashStackTrace gCrashStackTrace;
+
 #include <QCoreApplication>
 #include <csignal>
 #include <QObject>