From c944678c3cc79704d6e16a0790e3044c0fb334a2 Mon Sep 17 00:00:00 2001 From: Carlo von lynX Date: Fri, 29 Jul 2016 00:35:41 +0000 Subject: [PATCH] fixed a memleak, a static string free, an access of freed memory etc --- contrib/gnunet-logread | 4 ++-- src/cadet/gnunet-service-cadet_connection.c | 14 +++++++------- src/cadet/gnunet-service-cadet_peer.c | 7 ++++++- src/cadet/gnunet-service-cadet_tunnel.c | 1 + src/util/socks.c | 14 ++++++-------- 5 files changed, 22 insertions(+), 18 deletions(-) diff --git a/contrib/gnunet-logread b/contrib/gnunet-logread index f58758cb9..5b125a5a4 100755 --- a/contrib/gnunet-logread +++ b/contrib/gnunet-logread @@ -45,7 +45,7 @@ if (open HEADER, $filename) Could not read $filename for message codes: $!. Please provide a \$GNUNET_PREFIX environment variable to replace "/usr". -Try also '$0 -h' for help +Try also '$0 -h' for help. X } @@ -186,7 +186,7 @@ the GNUnet system beast. That master process is simply an extra gnunet-logread that you run in a separate window and adorn it with the '-f' flag. The submitting processes instead need to be given a '-n' flag. That is because from the GNUnet logs -it isn't clear which process events belong too. For example you may be +it isn't clear which process events belong to. For example you may be having events taking place in the 'util' subsystem of gnunet-psyc-service just as much as in the 'util' subsystem of gnunet-multicast-service. In order to make sense of them it is necessary to manually add that info. This diff --git a/src/cadet/gnunet-service-cadet_connection.c b/src/cadet/gnunet-service-cadet_connection.c index 9dfecf043..babb00d66 100644 --- a/src/cadet/gnunet-service-cadet_connection.c +++ b/src/cadet/gnunet-service-cadet_connection.c @@ -3482,6 +3482,13 @@ GCC_send_prebuilt_message (const struct GNUNET_MessageHeader *message, int droppable; GCC_check_connections (); + fc = fwd ? &c->fwd_fc : &c->bck_fc; + if (0 == fc->queue_max) + { + GNUNET_break (0); + return NULL; + } + size = ntohs (message->size); data = GNUNET_malloc (size); GNUNET_memcpy (data, message, size); @@ -3490,13 +3497,6 @@ GCC_send_prebuilt_message (const struct GNUNET_MessageHeader *message, "--> %s (%s %4u) on conn %s (%p) %s [%5u]\n", GC_m2s (type), GC_m2s (payload_type), payload_id, GCC_2s (c), c, GC_f2s(fwd), size); - - fc = fwd ? &c->fwd_fc : &c->bck_fc; - if (0 == fc->queue_max) - { - GNUNET_break (0); - return NULL; - } droppable = GNUNET_NO == force; switch (type) { diff --git a/src/cadet/gnunet-service-cadet_peer.c b/src/cadet/gnunet-service-cadet_peer.c index e19c3ca48..fa338f13f 100644 --- a/src/cadet/gnunet-service-cadet_peer.c +++ b/src/cadet/gnunet-service-cadet_peer.c @@ -787,7 +787,12 @@ peer_destroy (struct CadetPeer *peer) GNUNET_ATS_connectivity_suggest_cancel (peer->connectivity_suggestion); peer->connectivity_suggestion = NULL; } - while (NULL != peer->queue_head) + /* Following check was 'while' instead of 'if', but GCP_queue_destroy + * frees 'peer->queue_head' so the while checks on freed memory. + * Not sure if 'if' is what you wanted, but 'while' can't be + * correct. --lynX + */ + if (NULL != peer->queue_head) { GCP_queue_destroy (peer->queue_head, GNUNET_YES, GNUNET_NO, 0); } diff --git a/src/cadet/gnunet-service-cadet_tunnel.c b/src/cadet/gnunet-service-cadet_tunnel.c index bdb8a8ea4..565ddf411 100644 --- a/src/cadet/gnunet-service-cadet_tunnel.c +++ b/src/cadet/gnunet-service-cadet_tunnel.c @@ -4252,6 +4252,7 @@ GCT_send_connection_acks (struct CadetTunnel *t) { continue; } + GNUNET_assert(cs != 0); allow_per_connection = to_allow/cs; to_allow -= allow_per_connection; cs--; diff --git a/src/util/socks.c b/src/util/socks.c index 1525b3c75..387c2b698 100644 --- a/src/util/socks.c +++ b/src/util/socks.c @@ -575,10 +575,7 @@ GNUNET_SOCKS_do_connect (const char *service_name, GNUNET_CONFIGURATION_get_value_number (cfg, service_name, "SOCKSPORT", &port0)) port0 = 9050; /* A typical Tor client should usually try port 9150 for the TBB too, but - * GUNNet can probably assume a system Tor instalation. */ - if (GNUNET_OK != - GNUNET_CONFIGURATION_get_value_string (cfg, service_name, "SOCKSHOST", &host0)) - host0 = "127.0.0.1"; + * GUNNet can probably assume a system Tor installation. */ if (port0 > 65535 || port0 <= 0) { LOG (GNUNET_ERROR_TYPE_WARNING, @@ -587,7 +584,6 @@ GNUNET_SOCKS_do_connect (const char *service_name, port0,service_name); return NULL; } - if ((GNUNET_OK != GNUNET_CONFIGURATION_get_value_number (cfg, service_name, "PORT", &port1)) || (port1 > 65535) || (port1 <= 0) || @@ -600,9 +596,11 @@ GNUNET_SOCKS_do_connect (const char *service_name, service_name,port1,host1); return NULL; } - - socks5 = GNUNET_CONNECTION_create_from_connect (cfg, host0, port0); - GNUNET_free (host0); + if (GNUNET_OK != + GNUNET_CONFIGURATION_get_value_string (cfg, service_name, "SOCKSHOST", &host0)) + host0 = NULL; /* you don't want to feed a static string to free(), right? */ + socks5 = GNUNET_CONNECTION_create_from_connect (cfg, host0 || "127.0.0.1", port0); + if (host0) GNUNET_free (host0); /* Sets to NULL if they do not exist */ GNUNET_CONFIGURATION_get_value_string (cfg, service_name, "SOCKSUSER", &user); -- 2.25.1