From: Kirill Isakov Date: Fri, 23 Jul 2021 12:11:27 +0000 (+0600) Subject: Fix memory leaks triggered by integration tests. X-Git-Url: https://git.tinc-vpn.org/git/browse?a=commitdiff_plain;h=d72a450975bed625e058eb857410f0d78caee2d1;p=tinc Fix memory leaks triggered by integration tests. Found by AddressSanitizer and Valgrind. --- diff --git a/src/conf.c b/src/conf.c index ff592a86..4dd016cd 100644 --- a/src/conf.c +++ b/src/conf.c @@ -35,7 +35,7 @@ #include "utils.h" /* for cp */ #include "xalloc.h" -splay_tree_t *config_tree; +splay_tree_t *config_tree = NULL; int pinginterval = 0; /* seconds between pings */ int pingtimeout = 0; /* seconds to wait for response */ diff --git a/src/invitation.c b/src/invitation.c index 2f28aef4..d615976d 100644 --- a/src/invitation.c +++ b/src/invitation.c @@ -111,6 +111,9 @@ char *get_my_hostname() { scan_for_hostname(tinc_conf, &hostname, &port); } + free(name); + name = NULL; + if(hostname) { goto done; } @@ -407,6 +410,7 @@ int cmd_invite(int argc, char *argv[]) { if(!f) { fprintf(stderr, "Could not write %s: %s\n", filename, strerror(errno)); + free(key); return 1; } @@ -415,6 +419,7 @@ int cmd_invite(int argc, char *argv[]) { if(!ecdsa_write_pem_private_key(key, f)) { fprintf(stderr, "Could not write ECDSA private key\n"); fclose(f); + free(key); return 1; } @@ -444,6 +449,8 @@ int cmd_invite(int argc, char *argv[]) { sha512(fingerprint, strlen(fingerprint), hash); b64encode_urlsafe(hash, hash, 18); + free(key); + // Create a random cookie for this invitation. char cookie[25]; randomize(cookie, 18); @@ -456,6 +463,8 @@ int cmd_invite(int argc, char *argv[]) { sha512(buf, sizeof(buf), cookiehash); b64encode_urlsafe(cookiehash, cookiehash, 18); + free(fingerprint); + b64encode_urlsafe(cookie, cookie, 18); // Create a file containing the details of the invitation. @@ -1237,6 +1246,7 @@ int cmd_join(int argc, char *argv[]) { struct addrinfo *ai = str2addrinfo(address, port, SOCK_STREAM); if(!ai) { + free(b64key); return 1; } @@ -1250,6 +1260,7 @@ next: if(!aip) { freeaddrinfo(ai); + free(b64key); return 1; } } @@ -1296,6 +1307,11 @@ next: } freeaddrinfo(ai); + ai = NULL; + aip = NULL; + + free(b64key); + b64key = NULL; // Check if the hash of the key he gave us matches the hash in the URL. char *fingerprint = line + 2; diff --git a/src/names.c b/src/names.c index 603b5363..4f543c92 100644 --- a/src/names.c +++ b/src/names.c @@ -50,6 +50,8 @@ void make_names(bool daemon) { logger(DEBUG_ALWAYS, LOG_INFO, "Both netname and configuration directory given, using the latter..."); } + free(identname); + if(netname) { xasprintf(&identname, "tinc.%s", netname); } else { diff --git a/src/sptps_keypair.c b/src/sptps_keypair.c index 51a94eeb..51540aa5 100644 --- a/src/sptps_keypair.c +++ b/src/sptps_keypair.c @@ -97,12 +97,14 @@ int main(int argc, char *argv[]) { if(fp) { if(!ecdsa_write_pem_private_key(key, fp)) { fprintf(stderr, "Could not write ECDSA private key\n"); + free(key); return 1; } fclose(fp); } else { fprintf(stderr, "Could not open '%s' for writing: %s\n", argv[1], strerror(errno)); + free(key); return 1; } @@ -113,11 +115,12 @@ int main(int argc, char *argv[]) { fprintf(stderr, "Could not write ECDSA public key\n"); } + free(key); fclose(fp); + return 0; } else { fprintf(stderr, "Could not open '%s' for writing: %s\n", argv[2], strerror(errno)); + free(key); return 1; } - - return 0; } diff --git a/src/sptps_test.c b/src/sptps_test.c index 87f9b51b..64b8cb22 100644 --- a/src/sptps_test.c +++ b/src/sptps_test.c @@ -296,7 +296,6 @@ int main(int argc, char *argv[]) { int packetloss = 0; int r; int option_index = 0; - ecdsa_t *mykey = NULL, *hiskey = NULL; bool quit = false; while((r = getopt_long(argc, argv, "dqrstwL:W:v46", long_options, &option_index)) != EOF) { @@ -433,6 +432,7 @@ int main(int argc, char *argv[]) { if(sock < 0) { fprintf(stderr, "Could not create socket: %s\n", sockstrerror(sockerrno)); + freeaddrinfo(ai); return 1; } @@ -440,14 +440,24 @@ int main(int argc, char *argv[]) { setsockopt(sock, SOL_SOCKET, SO_REUSEADDR, (void *)&one, sizeof(one)); if(initiator) { - if(connect(sock, ai->ai_addr, ai->ai_addrlen)) { + int res = connect(sock, ai->ai_addr, ai->ai_addrlen); + + freeaddrinfo(ai); + ai = NULL; + + if(res) { fprintf(stderr, "Could not connect to peer: %s\n", sockstrerror(sockerrno)); return 1; } fprintf(stderr, "Connected\n"); } else { - if(bind(sock, ai->ai_addr, ai->ai_addrlen)) { + int res = bind(sock, ai->ai_addr, ai->ai_addrlen); + + freeaddrinfo(ai); + ai = NULL; + + if(res) { fprintf(stderr, "Could not bind socket: %s\n", sockstrerror(sockerrno)); return 1; } @@ -496,6 +506,8 @@ int main(int argc, char *argv[]) { return 1; } + ecdsa_t *mykey = NULL; + if(!(mykey = ecdsa_read_pem_private_key(fp))) { return 1; } @@ -506,10 +518,14 @@ int main(int argc, char *argv[]) { if(!fp) { fprintf(stderr, "Could not open %s: %s\n", argv[2], strerror(errno)); + free(mykey); return 1; } + ecdsa_t *hiskey = NULL; + if(!(hiskey = ecdsa_read_pem_public_key(fp))) { + free(mykey); return 1; } @@ -522,6 +538,8 @@ int main(int argc, char *argv[]) { sptps_t s; if(!sptps_start(&s, &sock, initiator, datagram, mykey, hiskey, "sptps_test", 10, send_data, receive_record)) { + free(mykey); + free(hiskey); return 1; } @@ -532,6 +550,8 @@ int main(int argc, char *argv[]) { if(in < 0) { fprintf(stderr, "Could not init stdin reader thread\n"); + free(mykey); + free(hiskey); return 1; } } @@ -558,6 +578,8 @@ int main(int argc, char *argv[]) { FD_SET(sock, &fds); if(select(max_fd + 1, &fds, NULL, NULL, NULL) <= 0) { + free(mykey); + free(hiskey); return 1; } @@ -570,6 +592,8 @@ int main(int argc, char *argv[]) { if(len < 0) { fprintf(stderr, "Could not read from stdin: %s\n", strerror(errno)); + free(mykey); + free(hiskey); return 1; } @@ -600,6 +624,8 @@ int main(int argc, char *argv[]) { sptps_send_record(&s, 0, buf, len); } } else if(!sptps_send_record(&s, buf[0] == '!' ? 1 : 0, buf, (len == 1 && buf[0] == '\n') ? 0 : buf[0] == '*' ? sizeof(buf) : (size_t)len)) { + free(mykey); + free(hiskey); return 1; } } @@ -609,6 +635,8 @@ int main(int argc, char *argv[]) { if(len < 0) { fprintf(stderr, "Could not read from socket: %s\n", sockstrerror(sockerrno)); + free(mykey); + free(hiskey); return 1; } @@ -638,6 +666,8 @@ int main(int argc, char *argv[]) { if(!done) { if(!datagram) { + free(mykey); + free(hiskey); return 1; } } @@ -648,7 +678,12 @@ int main(int argc, char *argv[]) { } } - if(!sptps_stop(&s)) { + bool stopped = sptps_stop(&s); + + free(mykey); + free(hiskey); + + if(!stopped) { return 1; } diff --git a/src/tincctl.c b/src/tincctl.c index 664b2eaa..57c42812 100644 --- a/src/tincctl.c +++ b/src/tincctl.c @@ -190,11 +190,13 @@ static bool parse_options(int argc, char **argv) { break; case 'c': /* config file */ + free(confbase); confbase = xstrdup(optarg); confbasegiven = true; break; case 'n': /* net name given */ + free(netname); netname = xstrdup(optarg); break; @@ -207,6 +209,7 @@ static bool parse_options(int argc, char **argv) { break; case 3: /* open control socket here */ + free(pidfilename); pidfilename = xstrdup(optarg); break; @@ -216,6 +219,7 @@ static bool parse_options(int argc, char **argv) { case '?': /* wrong options */ usage(true); + free_names(); return false; default: @@ -236,6 +240,7 @@ static bool parse_options(int argc, char **argv) { if(netname && (strpbrk(netname, "\\/") || *netname == '.')) { fprintf(stderr, "Invalid character in netname!\n"); + free_names(); return false; } @@ -1000,10 +1005,12 @@ static int cmd_start(int argc, char *argv[]) { #endif + char *default_c = "tincd"; + if(slash++) { xasprintf(&c, "%.*stincd", (int)(slash - program_name), program_name); } else { - c = "tincd"; + c = default_c; } int nargc = 0; @@ -1034,6 +1041,12 @@ static int cmd_start(int argc, char *argv[]) { #ifdef HAVE_MINGW int status = spawnvp(_P_WAIT, c, nargv); + free(nargv); + + if(c != default_c) { + free(c); + } + if(status == -1) { fprintf(stderr, "Error starting %s: %s\n", c, strerror(errno)); return 1; @@ -1046,6 +1059,11 @@ static int cmd_start(int argc, char *argv[]) { if(socketpair(AF_UNIX, SOCK_STREAM, 0, pfd)) { fprintf(stderr, "Could not create umbilical socket: %s\n", strerror(errno)); free(nargv); + + if(c != default_c) { + free(c); + } + return 1; } @@ -1054,6 +1072,11 @@ static int cmd_start(int argc, char *argv[]) { if(pid == -1) { fprintf(stderr, "Could not fork: %s\n", strerror(errno)); free(nargv); + + if(c != default_c) { + free(c); + } + return 1; } @@ -1103,12 +1126,17 @@ static int cmd_start(int argc, char *argv[]) { signal(SIGINT, SIG_DFL); #endif - if(failure || result != pid || !WIFEXITED(status) || WEXITSTATUS(status)) { + bool failed = failure || result != pid || !WIFEXITED(status) || WEXITSTATUS(status); + + if(failed) { fprintf(stderr, "Error starting %s\n", c); - return 1; } - return 0; + if(c != default_c) { + free(c); + } + + return failed ? EXIT_FAILURE : EXIT_SUCCESS; #endif } @@ -1963,6 +1991,11 @@ static int cmd_config(int argc, char *argv[]) { if(node && !check_id(node)) { fprintf(stderr, "Invalid name for node.\n"); + + if(node != line) { + free(node); + } + return 1; } @@ -1971,6 +2004,11 @@ static int cmd_config(int argc, char *argv[]) { fprintf(stderr, "Warning: %s is not a known configuration variable!\n", variable); } else { fprintf(stderr, "%s: is not a known configuration variable! Use --force to use it anyway.\n", variable); + + if(node && node != line) { + free(node); + } + return 1; } } @@ -1980,6 +2018,11 @@ static int cmd_config(int argc, char *argv[]) { if(node) { snprintf(filename, sizeof(filename), "%s" SLASH "%s", hosts_dir, node); + + if(node != line) { + free(node); + node = NULL; + } } else { snprintf(filename, sizeof(filename), "%s", tinc_conf); } diff --git a/src/tincd.c b/src/tincd.c index bb928b0d..e0e03885 100644 --- a/src/tincd.c +++ b/src/tincd.c @@ -159,6 +159,7 @@ static bool parse_options(int argc, char **argv) { break; case 'c': /* config file */ + free(confbase); confbase = xstrdup(optarg); break; @@ -169,7 +170,7 @@ static bool parse_options(int argc, char **argv) { case 'L': /* no detach */ #ifndef HAVE_MLOCKALL logger(DEBUG_ALWAYS, LOG_ERR, "The %s option is not supported on this platform.", argv[optind - 1]); - return false; + goto exit_fail; #else do_mlock = true; break; @@ -189,6 +190,7 @@ static bool parse_options(int argc, char **argv) { break; case 'n': /* net name given */ + free(netname); netname = xstrdup(optarg); break; @@ -201,7 +203,7 @@ static bool parse_options(int argc, char **argv) { cfg = parse_config_line(optarg, NULL, ++lineno); if(!cfg) { - return false; + goto exit_fail; } list_insert_tail(cmdline_conf, cfg); @@ -212,7 +214,7 @@ static bool parse_options(int argc, char **argv) { case 'R': case 'U': logger(DEBUG_ALWAYS, LOG_ERR, "The %s option is not supported on this platform.", argv[optind - 1]); - return false; + goto exit_fail; #else case 'R': /* chroot to NETNAME dir */ @@ -245,18 +247,20 @@ static bool parse_options(int argc, char **argv) { } if(optarg) { + free(logfilename); logfilename = xstrdup(optarg); } break; case 5: /* open control socket here */ + free(pidfilename); pidfilename = xstrdup(optarg); break; case '?': /* wrong options */ usage(true); - return false; + goto exit_fail; default: break; @@ -266,7 +270,7 @@ static bool parse_options(int argc, char **argv) { if(optind < argc) { fprintf(stderr, "%s: unrecognized argument '%s'\n", argv[0], argv[optind]); usage(true); - return false; + goto exit_fail; } if(!netname && (netname = getenv("NETNAME"))) { @@ -282,7 +286,7 @@ static bool parse_options(int argc, char **argv) { if(netname && !check_netname(netname, false)) { fprintf(stderr, "Invalid character in netname!\n"); - return false; + goto exit_fail; } if(netname && !check_netname(netname, true)) { @@ -290,6 +294,12 @@ static bool parse_options(int argc, char **argv) { } return true; + +exit_fail: + free_names(); + free(cmdline_conf); + cmdline_conf = NULL; + return false; } static bool drop_privs(void) { @@ -372,6 +382,15 @@ static BOOL WINAPI console_ctrl_handler(DWORD type) { # define setpriority(level) (setpriority(PRIO_PROCESS, 0, (level))) #endif +static void cleanup() { + if(config_tree) { + exit_configuration(&config_tree); + } + + free(cmdline_conf); + free_names(); +} + int main(int argc, char **argv) { program_name = argv[0]; @@ -432,6 +451,8 @@ int main(int argc, char **argv) { } make_names(true); + atexit(cleanup); + chdir(confbase); #ifdef HAVE_MINGW @@ -607,9 +628,5 @@ end: crypto_exit(); - exit_configuration(&config_tree); - free(cmdline_conf); - free_names(); - return status; }