diff mbox series

[1/5] wlantest: properly free allocated mem and stop abrupt return without deinit

Message ID 20211103165024.3634375-1-gokulkumar792@gmail.com
State Accepted
Headers show
Series [1/5] wlantest: properly free allocated mem and stop abrupt return without deinit | expand

Commit Message

Gokul Sivakumar Nov. 3, 2021, 4:50 p.m. UTC 
In the cases when a failure is experienced, the value "-1" is returned from
the main function without doing any cleanup or deinit.

For Example, If wlantest is started with the following set of cmd line args
then later when returning after a failure from main function, the memory
allocated as part of handling the "-p" getopt cmd line option is not freed.
To fix memleaks in this case, properly free the previously allocated memory
with the help of wlantest_deinit() before returning from main.

$ sudo valgrind --leak-check=full --show-leak-kinds=all --verbose \
> --track-origins=yes --log-file=valgrind-out.txt \
> ./wlantest -i hwsim0 -dd -c -p "asdfasdfasdfasdf" -W "abcd"
Invalid WEP key 'abcd'

Memory leak reported by Valgrind when running wlantest as mentioned above.

==513454== HEAP SUMMARY:
==513454==     in use at exit: 128 bytes in 1 blocks
==513454==   total heap usage: 4 allocs, 3 frees, 5,720 bytes allocated
==513454==
==513454== Searching for pointers to 1 not-freed blocks
==513454== Checked 76,936 bytes
==513454==
==513454== 128 bytes in 1 blocks are definitely lost in loss record 1 of 1
==513454==    at 0x483DD99: calloc (in /usr/lib/x86_64-linux-gnu/valgrind/vgpreload_memcheck-amd64-linux.so)
==513454==    by 0x1396CA: os_zalloc (in /home/ubuntu/hostap/wlantest/wlantest)
==513454==    by 0x10C345: add_passphrase (wlantest.c:125)
==513454==    by 0x10C345: main (wlantest.c:425)
==513454==
==513454== LEAK SUMMARY:
==513454==    definitely lost: 128 bytes in 1 blocks
==513454==    indirectly lost: 0 bytes in 0 blocks
==513454==      possibly lost: 0 bytes in 0 blocks
==513454==    still reachable: 0 bytes in 0 blocks
==513454==         suppressed: 0 bytes in 0 blocks
==513454==
==513454== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

Signed-off-by: Gokul Sivakumar <gokulkumar792@gmail.com>
---
 wlantest/wlantest.c | 81 ++++++++++++++++++++++++---------------------
 1 file changed, 44 insertions(+), 37 deletions(-)

Comments

Jouni Malinen Dec. 11, 2021, 8:58 p.m. UTC | #1 
Thanks, all five patches applied.
diff mbox series

Patch

diff --git a/wlantest/wlantest.c b/wlantest/wlantest.c
index ac20b068e..ae4eff73c 100644
--- a/wlantest/wlantest.c
+++ b/wlantest/wlantest.c
@@ -364,20 +364,22 @@  int wlantest_relog(struct wlantest *wt)
 
 int main(int argc, char *argv[])
 {
-	int c;
+	int c, ret = 0;
 	const char *read_file = NULL;
 	const char *read_wired_file = NULL;
 	const char *ifname = NULL;
 	const char *ifname_wired = NULL;
 	const char *logfile = NULL;
 	struct wlantest wt;
-	int ctrl_iface = 0;
+	int ctrl_iface = 0, eloop_init_done = 0;
 
 	wpa_debug_level = MSG_INFO;
 	wpa_debug_show_keys = 1;
 
-	if (os_program_init())
-		return -1;
+	if (os_program_init()) {
+		ret = -1;
+		goto exit;
+	}
 
 	wlantest_init(&wt);
 
@@ -397,15 +399,18 @@  int main(int argc, char *argv[])
 			wt.ethernet = 1;
 			break;
 		case 'f':
-			if (add_pmk_file(&wt, optarg) < 0)
-				return -1;
+			if (add_pmk_file(&wt, optarg) < 0) {
+				ret = -1;
+				goto deinit;
+			}
 			break;
 		case 'F':
 			wt.assume_fcs = 1;
 			break;
 		case 'h':
 			usage();
-			return 0;
+			ret = 0;
+			goto deinit;
 		case 'i':
 			ifname = optarg;
 			break;
@@ -440,54 +445,53 @@  int main(int argc, char *argv[])
 			wpa_debug_timestamp = 1;
 			break;
 		case 'T':
-			if (add_ptk_file(&wt, optarg) < 0)
-				return -1;
+			if (add_ptk_file(&wt, optarg) < 0) {
+				ret = -1;
+				goto deinit;
+			}
 			break;
 		case 'w':
 			wt.write_file = optarg;
 			break;
 		case 'W':
-			if (add_wep(&wt, optarg) < 0)
-				return -1;
+			if (add_wep(&wt, optarg) < 0) {
+				ret = -1;
+				goto deinit;
+			}
 			break;
 		default:
 			usage();
-			return -1;
+			ret = -1;
+			goto deinit;
 		}
 	}
 
 	if (ifname == NULL && ifname_wired == NULL &&
 	    read_file == NULL && read_wired_file == NULL) {
 		usage();
-		return 0;
+		ret = 0;
+		goto deinit;
 	}
 
-	if (eloop_init())
-		return -1;
+	if (eloop_init()) {
+		ret = -1;
+		goto deinit;
+	}
+	eloop_init_done = 1;
 
 	if (logfile)
 		wpa_debug_open_file(logfile);
 
-	if (wt.write_file && write_pcap_init(&wt, wt.write_file) < 0)
-		return -1;
-
-	if (wt.pcapng_file && write_pcapng_init(&wt, wt.pcapng_file) < 0)
-		return -1;
-
-	if (read_wired_file && read_wired_cap_file(&wt, read_wired_file) < 0)
-		return -1;
-
-	if (read_file && read_cap_file(&wt, read_file) < 0)
-		return -1;
-
-	if (ifname && monitor_init(&wt, ifname) < 0)
-		return -1;
-
-	if (ifname_wired && monitor_init_wired(&wt, ifname_wired) < 0)
-		return -1;
-
-	if (ctrl_iface && ctrl_init(&wt) < 0)
-		return -1;
+	if ((wt.write_file && write_pcap_init(&wt, wt.write_file) < 0) ||
+	    (wt.pcapng_file && write_pcapng_init(&wt, wt.pcapng_file) < 0) ||
+	    (read_wired_file && read_wired_cap_file(&wt, read_wired_file) < 0) ||
+	    (read_file && read_cap_file(&wt, read_file) < 0) ||
+	    (ifname && monitor_init(&wt, ifname) < 0) ||
+	    (ifname_wired && monitor_init_wired(&wt, ifname_wired) < 0) ||
+	    (ctrl_iface && ctrl_init(&wt) < 0)) {
+		ret = -1;
+		goto deinit;
+	}
 
 	eloop_register_signal_terminate(wlantest_terminate, &wt);
 
@@ -497,11 +501,14 @@  int main(int argc, char *argv[])
 		   "fcs_error=%u",
 		   wt.rx_mgmt, wt.rx_ctrl, wt.rx_data, wt.fcs_error);
 
+deinit:
 	wlantest_deinit(&wt);
 
 	wpa_debug_close_file();
-	eloop_destroy();
+	if (eloop_init_done)
+		eloop_destroy();
 	os_program_deinit();
 
-	return 0;
+exit:
+	return ret;
 }