From f52e590232f939bd6b5c88a483765c03e37fc733 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Tue, 15 Dec 2020 09:14:09 -0500 Subject: [PATCH] Fix: event notifier group: fix fd leak on error Use a regular pattern for all commands: If the command callback takes ownership of a pointer or file descriptor, it sets them to NULL or -1. Therefore, the caller can always try to free the pointer, or close it if it is greater or equal to 0. This eliminates memory and fd leaks on error. Change-Id: If5181a0c438f2e1ac73e5d4d63d5cde21cc97e52 Signed-off-by: Mathieu Desnoyers --- liblttng-ust/lttng-ust-abi.c | 19 ++++++++----------- liblttng-ust/lttng-ust-comm.c | 9 ++++++++- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c index 77bb90db..7490f8b7 100644 --- a/liblttng-ust/lttng-ust-abi.c +++ b/liblttng-ust/lttng-ust-abi.c @@ -346,10 +346,10 @@ long lttng_abi_tracer_version(int objd, } static -int lttng_abi_event_notifier_send_fd(void *owner, int event_notifier_notif_fd) +int lttng_abi_event_notifier_send_fd(void *owner, int *event_notifier_notif_fd) { struct lttng_event_notifier_group *event_notifier_group; - int event_notifier_group_objd, ret, fd_flag, close_ret; + int event_notifier_group_objd, ret, fd_flag; event_notifier_group = lttng_event_notifier_group_create(); if (!event_notifier_group) @@ -358,11 +358,11 @@ int lttng_abi_event_notifier_send_fd(void *owner, int event_notifier_notif_fd) /* * Set this file descriptor as NON-BLOCKING. */ - fd_flag = fcntl(event_notifier_notif_fd, F_GETFL); + fd_flag = fcntl(*event_notifier_notif_fd, F_GETFL); fd_flag |= O_NONBLOCK; - ret = fcntl(event_notifier_notif_fd, F_SETFL, fd_flag); + ret = fcntl(*event_notifier_notif_fd, F_SETFL, fd_flag); if (ret) { ret = -errno; goto fd_error; @@ -377,18 +377,15 @@ int lttng_abi_event_notifier_send_fd(void *owner, int event_notifier_notif_fd) event_notifier_group->objd = event_notifier_group_objd; event_notifier_group->owner = owner; - event_notifier_group->notification_fd = event_notifier_notif_fd; + event_notifier_group->notification_fd = *event_notifier_notif_fd; + /* Object descriptor takes ownership of notification fd. */ + *event_notifier_notif_fd = -1; return event_notifier_group_objd; objd_error: lttng_event_notifier_group_destroy(event_notifier_group); fd_error: - close_ret = close(event_notifier_notif_fd); - if (close_ret) { - PERROR("close"); - } - return ret; } @@ -443,7 +440,7 @@ long lttng_cmd(int objd, unsigned int cmd, unsigned long arg, return 0; case LTTNG_UST_EVENT_NOTIFIER_GROUP_CREATE: return lttng_abi_event_notifier_send_fd(owner, - uargs->event_notifier_handle.event_notifier_notif_fd); + &uargs->event_notifier_handle.event_notifier_notif_fd); default: return -EINVAL; } diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c index 708b9192..7e3bf450 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -987,7 +987,7 @@ int handle_message(struct sock_info *sock_info, } case LTTNG_UST_EVENT_NOTIFIER_GROUP_CREATE: { - int event_notifier_notif_fd; + int event_notifier_notif_fd, close_ret; len = ustcomm_recv_event_notifier_notif_fd_from_sessiond(sock, &event_notifier_notif_fd); @@ -1024,6 +1024,13 @@ int handle_message(struct sock_info *sock_info, &args, sock_info); else ret = -ENOSYS; + if (args.event_notifier_handle.event_notifier_notif_fd >= 0) { + lttng_ust_lock_fd_tracker(); + close_ret = close(args.event_notifier_handle.event_notifier_notif_fd); + lttng_ust_unlock_fd_tracker(); + if (close_ret) + PERROR("close"); + } break; } case LTTNG_UST_CHANNEL: -- 2.34.1