Fix: refcount issue in lttng-ust-abi.c

The following scenario could lead to a segmentation fault in
applications when the sessiond disappears or when the application try to
exit. This is caused by incorrect handling of reference counts.

This can happen in very particular race scenario, described as follows:

1) The sessiond asks for "release" of one or more objects (e.g. a
   session object), but _without_ asking for release of _all_ objects
   referencing the session.
2) The application exits, thus calling objd_table_destroy(). It walks on
   all objects, decrementing their reference count, freeing memory when
   their reference count reaches "1".

However, here is the issue: since "release" has already been performed
by sessiond on the session object, this extra reference count unref
performed by objd_table_destroy() can make the session object disappear
while it is still needed by either the channel object or the enabler
object. Therefore, we can experience a segmentation fault when we try to
unref and free the channel or enabler objects within
objd_table_destroy().

Fix this issue by adding the concept of an "owner reference". Only
objd_table_destroy(), lttng_ust_objd_table_owner_cleanup(), "release"
commands, and failure paths of object creation are allowed to decrement
the owner reference. We restrict objd_table_destroy() and
lttng_ust_objd_table_owner_cleanup() to _only_ decrement refcount of
objects _if_ their owner reference is still held.

Signed-off-by: Mathieu Desnoyers <mathieu.desnoyers@efficios.com>

diff --git a/include/lttng/ust-abi.h b/include/lttng/ust-abi.h
index a5740357c32ae3afb55348b2ae278fb533f48000..d7df774486735713d81d7beb8beac02ae1209fa5 100644 (file)
--- a/include/lttng/ust-abi.h
+++ b/include/lttng/ust-abi.h
@@ -269,7 +269,7 @@ struct lttng_ust_objd_ops {
 int lttng_abi_create_root_handle(void);
 
 const struct lttng_ust_objd_ops *objd_ops(int id);
-int lttng_ust_objd_unref(int id);
+int lttng_ust_objd_unref(int id, int is_owner);
 
 void lttng_ust_abi_exit(void);
 void lttng_ust_events_exit(void);

diff --git a/liblttng-ust/lttng-ust-abi.c b/liblttng-ust/lttng-ust-abi.c
index 1985d01864282200fe2a42aadf69a8f0b7a71bbe..ddadfc1d7f33c9a4fb9310976af296f33bdd66db 100644 (file)
--- a/liblttng-ust/lttng-ust-abi.c
+++ b/liblttng-ust/lttng-ust-abi.c
@@ -69,6 +69,7 @@ struct lttng_ust_obj {
                        void *private_data;
                        const struct lttng_ust_objd_ops *ops;
                        int f_count;
+                       int owner_ref;  /* has ref from owner */
                        void *owner;
                        char name[OBJ_NAME_LEN];
                } s;
@@ -124,6 +125,7 @@ end:
        obj->u.s.ops = ops;
        obj->u.s.f_count = 2;   /* count == 1 : object is allocated */
                                /* count == 2 : allocated + hold ref */
+       obj->u.s.owner_ref = 1; /* One owner reference */
        obj->u.s.owner = owner;
        strncpy(obj->u.s.name, name, OBJ_NAME_LEN);
        obj->u.s.name[OBJ_NAME_LEN - 1] = '\0';
@@ -184,7 +186,7 @@ void objd_ref(int id)
        obj->u.s.f_count++;
 }
 
-int lttng_ust_objd_unref(int id)
+int lttng_ust_objd_unref(int id, int is_owner)
 {
        struct lttng_ust_obj *obj = _objd_get(id);
 
@@ -194,6 +196,13 @@ int lttng_ust_objd_unref(int id)
                ERR("Reference counting error\n");
                return -EINVAL;
        }
+       if (is_owner) {
+               if (!obj->u.s.owner_ref) {
+                       ERR("Error decrementing owner reference");
+                       return -EINVAL;
+               }
+               obj->u.s.owner_ref--;
+       }
        if ((--obj->u.s.f_count) == 1) {
                const struct lttng_ust_objd_ops *ops = objd_ops(id);
                
@@ -209,8 +218,16 @@ void objd_table_destroy(void)
 {
        int i;
 
-       for (i = 0; i < objd_table.allocated_len; i++)
-               (void) lttng_ust_objd_unref(i);
+       for (i = 0; i < objd_table.allocated_len; i++) {
+               struct lttng_ust_obj *obj;
+
+               obj = _objd_get(i);
+               if (!obj)
+                       continue;
+               if (!obj->u.s.owner_ref)
+                       continue;       /* only unref owner ref. */
+               (void) lttng_ust_objd_unref(i, 1);
+       }
        free(objd_table.array);
        objd_table.array = NULL;
        objd_table.len = 0;
@@ -230,8 +247,10 @@ void lttng_ust_objd_table_owner_cleanup(void *owner)
                        continue;
                if (!obj->u.s.owner)
                        continue;       /* skip root handles */
+               if (!obj->u.s.owner_ref)
+                       continue;       /* only unref owner ref. */
                if (obj->u.s.owner == owner)
-                       (void) lttng_ust_objd_unref(i);
+                       (void) lttng_ust_objd_unref(i, 1);
        }
 }
 
@@ -461,7 +480,7 @@ chan_error:
        {
                int err;
 
-               err = lttng_ust_objd_unref(chan_objd);
+               err = lttng_ust_objd_unref(chan_objd, 1);
                assert(!err);
        }
 objd_error:
@@ -598,7 +617,7 @@ alloc_error:
        {
                int err;
 
-               err = lttng_ust_objd_unref(list_objd);
+               err = lttng_ust_objd_unref(list_objd, 1);
                assert(!err);
        }
 objd_error:
@@ -681,7 +700,7 @@ alloc_error:
        {
                int err;
 
-               err = lttng_ust_objd_unref(list_objd);
+               err = lttng_ust_objd_unref(list_objd, 1);
                assert(!err);
        }
 objd_error:
@@ -785,7 +804,7 @@ event_error:
        {
                int err;
 
-               err = lttng_ust_objd_unref(event_objd);
+               err = lttng_ust_objd_unref(event_objd, 1);
                assert(!err);
        }
 objd_error:
@@ -902,7 +921,7 @@ int lttng_channel_release(int objd)
        struct lttng_channel *channel = objd_private(objd);
 
        if (channel)
-               return lttng_ust_objd_unref(channel->session->objd);
+               return lttng_ust_objd_unref(channel->session->objd, 0);
        return 0;
 }
 
@@ -965,7 +984,7 @@ int lttng_rb_release(int objd)
                if (!lttng_ust_abi_close_in_progress)
                        channel->ops->buffer_read_close(buf, channel->handle);
 
-               return lttng_ust_objd_unref(channel->objd);
+               return lttng_ust_objd_unref(channel->objd, 0);
        }
        return 0;
 }
@@ -1030,7 +1049,7 @@ int lttng_enabler_release(int objd)
        struct lttng_enabler *enabler = objd_private(objd);
 
        if (enabler)
-               return lttng_ust_objd_unref(enabler->chan->objd);
+               return lttng_ust_objd_unref(enabler->chan->objd, 0);
        return 0;
 }
 

diff --git a/liblttng-ust/lttng-ust-comm.c b/liblttng-ust/lttng-ust-comm.c
index d7214370821d05b459a49a188b69734ab58139b7..946584a3898ff616044b222bdfffd9d1d620427e 100644 (file)
--- a/liblttng-ust/lttng-ust-comm.c
+++ b/liblttng-ust/lttng-ust-comm.c
@@ -286,7 +286,7 @@ int handle_message(struct sock_info *sock_info,
                if (lum->handle == LTTNG_UST_ROOT_HANDLE)
                        ret = -EPERM;
                else
-                       ret = lttng_ust_objd_unref(lum->handle);
+                       ret = lttng_ust_objd_unref(lum->handle, 1);
                break;
        case LTTNG_UST_FILTER:
        {
@@ -549,7 +549,7 @@ void cleanup_sock_info(struct sock_info *sock_info, int exiting)
                sock_info->socket = -1;
        }
        if (sock_info->root_handle != -1) {
-               ret = lttng_ust_objd_unref(sock_info->root_handle);
+               ret = lttng_ust_objd_unref(sock_info->root_handle, 1);
                if (ret) {
                        ERR("Error unref root handle");
                }