Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
From:   Tedd Ho-Jeong An <hj.tedd.an@gmail.com>
To:     linux-bluetooth@vger.kernel.org
Cc:     tedd.an@intel.com
Subject: [PATCH 1/6] monitor: Fix potential memory leak
Date:   Fri, 20 Nov 2020 12:07:07 -0800
Message-Id: <20201120200712.491219-1-tedd.an@intel.com>
Reply-To: tedd.an@intel.com
MIME-Version: 1.0
Content-Transfer-Encoding: 8bit
Precedence: bulk

If the mainloop_add_fd() returns with failure, the destroy callback is
never called so any reosurces need to be released never freed/closed.

This potential leakage is checked with valgrind after failing the
mainloop_add_fd() function manually.

   ==258684== 1,500 bytes in 1 blocks are definitely lost in loss record 3 of 3
   ==258684==    at 0x483BB1A: calloc (vg_replace_malloc.c:760)
   ==258684==    by 0x123F1A: open_channel (control.c:1058)
   ==258684==    by 0x125B09: control_tracing (control.c:1540)
   ==258684==    by 0x122764: main (main.c:255)
   ==258684==
   ==258684== LEAK SUMMARY:
   ==258684==    definitely lost: 1,500 bytes in 1 blocks
   ==258684==    indirectly lost: 0 bytes in 0 blocks
   ==258684==      possibly lost: 0 bytes in 0 blocks
   ==258684==    still reachable: 48 bytes in 2 blocks
   ==258684==         suppressed: 0 bytes in 0 blocks

This patch frees/closes the resources if the function returns with
failure.
---
 monitor/control.c | 20 +++++++++++++++++---
 monitor/hcidump.c | 14 +++++++++++---
 2 files changed, 28 insertions(+), 6 deletions(-)

diff --git a/monitor/control.c b/monitor/control.c
index 962da4980..d1ba97d37 100644
--- a/monitor/control.c
+++ b/monitor/control.c
@@ -1071,7 +1071,12 @@ static int open_channel(uint16_t channel)
 	if (filter_index != HCI_DEV_NONE)
 		attach_index_filter(data->fd, filter_index);
 
-	mainloop_add_fd(data->fd, EPOLLIN, data_callback, data, free_data);
+	if (mainloop_add_fd(data->fd, EPOLLIN, data_callback,
+						data, free_data) < 0) {
+		close(data->fd);
+		free(data);
+		return -1;
+	};
 
 	return 0;
 }
@@ -1148,7 +1153,11 @@ static void server_accept_callback(int fd, uint32_t events, void *user_data)
 	data->channel = HCI_CHANNEL_MONITOR;
 	data->fd = nfd;
 
-        mainloop_add_fd(data->fd, EPOLLIN, client_callback, data, free_data);
+	if (mainloop_add_fd(data->fd, EPOLLIN, client_callback,
+						data, free_data) < 0) {
+		close(data->fd);
+		free(data);
+	}
 }
 
 static int server_fd = -1;
@@ -1399,7 +1408,12 @@ int control_tty(const char *path, unsigned int speed)
 	data->channel = HCI_CHANNEL_MONITOR;
 	data->fd = fd;
 
-	mainloop_add_fd(data->fd, EPOLLIN, tty_callback, data, free_data);
+	if (mainloop_add_fd(data->fd, EPOLLIN, tty_callback,
+						data, free_data) < 0) {
+		close(data->fd);
+		free(data);
+		return -1;
+	}
 
 	return 0;
 }
diff --git a/monitor/hcidump.c b/monitor/hcidump.c
index 690b9b913..fac9c8a08 100644
--- a/monitor/hcidump.c
+++ b/monitor/hcidump.c
@@ -184,7 +184,11 @@ static void open_device(uint16_t index)
 		return;
 	}
 
-	mainloop_add_fd(data->fd, EPOLLIN, device_callback, data, free_data);
+	if (mainloop_add_fd(data->fd, EPOLLIN, device_callback,
+						data, free_data) < 0) {
+		close(data->fd);
+		free(data);
+	}
 }
 
 static void device_info(int fd, uint16_t index, uint8_t *type, uint8_t *bus,
@@ -393,8 +397,12 @@ int hcidump_tracing(void)
 		return -1;
 	}
 
-	mainloop_add_fd(data->fd, EPOLLIN, stack_internal_callback,
-							data, free_data);
+	if (mainloop_add_fd(data->fd, EPOLLIN, stack_internal_callback,
+							data, free_data) < 0) {
+		close(data->fd);
+		free(data);
+		return -1;
+	}
 
 	return 0;
 }
-- 
2.25.4