From 1849ef7cdc349bef4b6f6eb4a50058e06c26d431 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Wed, 6 Mar 2013 10:07:12 -0500 Subject: [PATCH] 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 --- include/lttng/ust-abi.h | 2 +- liblttng-ust/lttng-ust-abi.c | 37 ++++++++++++++++++++++++++--------- liblttng-ust/lttng-ust-comm.c | 4 ++-- 3 files changed, 31 insertions(+), 12 deletions(-) diff --git a/include/lttng/ust-abi.h b/include/lttng/ust-abi.h index 3c9f4600..8f2233a5 100644 --- a/include/lttng/ust-abi.h +++ b/include/lttng/ust-abi.h @@ -299,7 +299,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 94c05968..70ec22aa 100644 --- a/liblttng-ust/lttng-ust-abi.c +++ b/liblttng-ust/lttng-ust-abi.c @@ -71,6 +71,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; @@ -126,6 +127,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'; @@ -186,7 +188,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); @@ -196,6 +198,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); @@ -211,8 +220,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; @@ -241,8 +258,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); } } @@ -608,7 +627,7 @@ alloc_error: { int err; - err = lttng_ust_objd_unref(list_objd); + err = lttng_ust_objd_unref(list_objd, 1); assert(!err); } objd_error: @@ -688,7 +707,7 @@ alloc_error: { int err; - err = lttng_ust_objd_unref(list_objd); + err = lttng_ust_objd_unref(list_objd, 1); assert(!err); } objd_error: @@ -767,7 +786,7 @@ event_error: { int err; - err = lttng_ust_objd_unref(event_objd); + err = lttng_ust_objd_unref(event_objd, 1); assert(!err); } objd_error: @@ -855,7 +874,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; } @@ -919,7 +938,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 874adde6..096a22fc 100644 --- a/liblttng-ust/lttng-ust-comm.c +++ b/liblttng-ust/lttng-ust-comm.c @@ -393,7 +393,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: { @@ -632,7 +632,7 @@ void cleanup_sock_info(struct sock_info *sock_info, int exiting) sock_info->notify_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"); } -- 2.34.1