diff mbox series

[1/6] monitor: Fix potential memory leak

Message ID 20201120200712.491219-1-tedd.an@intel.com (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series [1/6] monitor: Fix potential memory leak | expand

Commit Message

Tedd Ho-Jeong An Nov. 20, 2020, 8:07 p.m. UTC 
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(-)

Comments

bluez.test.bot@gmail.com Nov. 20, 2020, 8:27 p.m. UTC | #1 
This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=388665

---Test result---

##############################
Test: CheckPatch - FAIL
Output:
monitor: Fix potential memory leak
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#12: 
   ==258684== 1,500 bytes in 1 blocks are definitely lost in loss record 3 of 3

- total: 0 errors, 1 warnings, 64 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

"[PATCH] monitor: Fix potential memory leak" has style problems, please review.

NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckGitLint - PASS

##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth
Luiz Augusto von Dentz Nov. 24, 2020, 9:22 p.m. UTC | #2 
Hi Tedd,

On Fri, Nov 20, 2020 at 12:31 PM <bluez.test.bot@gmail.com> wrote:
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=388665
>
> ---Test result---
>
> ##############################
> Test: CheckPatch - FAIL
> Output:
> monitor: Fix potential memory leak
> WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
> #12:
>    ==258684== 1,500 bytes in 1 blocks are definitely lost in loss record 3 of 3
>
> - total: 0 errors, 1 warnings, 64 lines checked
>
> NOTE: For some of the reported defects, checkpatch may be able to
>       mechanically convert to the typical style using --fix or --fix-inplace.
>
> "[PATCH] monitor: Fix potential memory leak" has style problems, please review.
>
> NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING SSCANF_TO_KSTRTO
>
> NOTE: If any of the errors are false positives, please report
>       them to the maintainer, see CHECKPATCH in MAINTAINERS.
>
>
> ##############################
> Test: CheckGitLint - PASS
>
> ##############################
> Test: CheckBuild - PASS
>
> ##############################
> Test: MakeCheck - PASS
>
>
>
> ---
> Regards,
> Linux Bluetooth

Applied, thanks.
diff mbox series

Patch

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;
 }