Fix: channel 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: I0f9cba5fc0f4c095c8ec8f3e8970de8a10386876
Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c
index 2de4df1aeee9a3e0dd992e2843681cff070e0c9f..daf0d5e4caa0185652605e9a0496c2ad0fb2f0b3 100644 (file)
--- a/liblttng-ust/lttng-ust-abi.c
+++ b/liblttng-ust/lttng-ust-abi.c
@@ -438,6 +438,10 @@ int lttng_abi_map_channel(int session_objd,
                goto handle_error;
        }
 
+       /* Ownership of chan_data and wakeup_fd taken by channel handle. */
+       uargs->channel.chan_data = NULL;
+       uargs->channel.wakeup_fd = -1;
+
        chan = shmp(channel_handle, channel_handle->chan);
        assert(chan);
        chan->handle = channel_handle;
@@ -521,24 +525,9 @@ alloc_error:
        channel_destroy(chan, channel_handle, 0);
        return ret;
 
-       /*
-        * error path before channel creation (owning chan_data and
-        * wakeup_fd).
-        */
 handle_error:
 active:
 invalid:
-       {
-               int close_ret;
-
-               lttng_ust_lock_fd_tracker();
-               close_ret = close(wakeup_fd);
-               lttng_ust_unlock_fd_tracker();
-               if (close_ret) {
-                       PERROR("close");
-               }
-       }
-       free(chan_data);
        return ret;
 }
 

diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index 9d0c010d502079a164e453cb67778f3a547cc2d1..3d60596a9896ef345a5e29b17bf91e051ba3c1cb 100644 (file)
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -943,6 +943,17 @@ int handle_message(struct sock_info *sock_info,
                                        &args, sock_info);
                else
                        ret = -ENOSYS;
+               if (args.channel.wakeup_fd >= 0) {
+                       int close_ret;
+
+                       lttng_ust_lock_fd_tracker();
+                       close_ret = close(args.channel.wakeup_fd);
+                       lttng_ust_unlock_fd_tracker();
+                       args.channel.wakeup_fd = -1;
+                       if (close_ret)
+                               PERROR("close");
+               }
+               free(args.channel.chan_data);
                break;
        }
        case LTTNG_UST_STREAM: