A session teardown can be initiated by a dying application. Hence, a
session object can exist without a valid registry. As a result,
get_session_registry can return null. To prevent this, the UST
application session lock should be held, when possible, when looking up
the registry to ensure synchronization. Otherwise the presence of a
registry is not guaranteed. In such case, handling a null return value
from look-up registry function is necessary.
Core dumps, triggered by the "assert(registry)" statement found in
reply_ust_register_channel, were observed when killing instrumented
applications. In this occurrence, obtaining the UST application lock
result in a deadlock since the lock is already held during
ust_app_global_create. Handling the null value is simpler and
corresponds with the handling of previous look-up done during the
function.
Handling of null value is also applied to:
add_event_ust_registry
add_enum_ust_registry
ust_app_snapshot_record
Signed-off-by: Jonathan Rajotte <jonathan.rajotte-jul...@efficios.com>
---
src/bin/lttng-sessiond/ust-app.c | 53 ++++++++++++++++++++++++++++++++++------
1 file changed, 45 insertions(+), 8 deletions(-)
diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index 5a41c38..4395a59 100644
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -810,6 +810,7 @@ void delete_ust_app_session(int sock, struct
ust_app_session *ua_sess,
ua_sess->deleted = true;
registry = get_session_registry(ua_sess);
+ /* Registry can be null on error path during initialization */
if (registry) {
/* Push metadata for application before freeing the
application. */
(void) push_metadata(registry, ua_sess->consumer);
@@ -837,6 +838,10 @@ void delete_ust_app_session(int sock, struct
ust_app_session *ua_sess,
if (ua_sess->buffer_type == LTTNG_BUFFER_PER_PID) {
struct buffer_reg_pid *reg_pid =
buffer_reg_pid_find(ua_sess->id);
if (reg_pid) {
+ /*
+ * Registry can be null on error path during
+ * initialization
+ */
buffer_reg_pid_remove(reg_pid);
buffer_reg_pid_destroy(reg_pid);
}
@@ -2464,6 +2469,8 @@ error:
/*
* Ask the consumer to create a channel and get it if successful.
*
+ * Called with UST app session lock held.
+ *
* Return 0 on success or else a negative value.
*/
static int do_consumer_create_channel(struct ltt_ust_session *usess,
@@ -2917,6 +2924,8 @@ error:
/*
* Create and send to the application the created buffers with per PID buffers.
*
+ * Called with UST app session lock held.
+ *
* Return 0 on success else a negative value.
*/
static int create_channel_per_pid(struct ust_app *app,
@@ -2936,6 +2945,7 @@ static int create_channel_per_pid(struct ust_app *app,
rcu_read_lock();
registry = get_session_registry(ua_sess);
+ /* The UST app session lock is held, registry shall not be null */
assert(registry);
/* Create and add a new channel registry to session. */
@@ -2973,6 +2983,8 @@ error:
* need and send it to the application. This MUST be called with a RCU read
* side lock acquired.
*
+ * Called with UST app session lock held.
+ *
* Return 0 on success or else a negative value. Returns -ENOTCONN if
* the application exited concurrently.
*/
@@ -3160,6 +3172,7 @@ static int create_ust_app_metadata(struct ust_app_session
*ua_sess,
assert(consumer);
registry = get_session_registry(ua_sess);
+ /* The UST app session is held registry shall not be null */
assert(registry);
pthread_mutex_lock(®istry->lock);
@@ -4296,6 +4309,9 @@ int ust_app_create_event_glb(struct ltt_ust_session
*usess,
/*
* Start tracing for a specific UST session and app.
+ *
+ * Called with UST app session lock held.
+ *
*/
static
int ust_app_start_trace(struct ltt_ust_session *usess, struct ust_app *app)
@@ -4479,6 +4495,8 @@ int ust_app_stop_trace(struct ltt_ust_session *usess,
struct ust_app *app)
health_code_update();
registry = get_session_registry(ua_sess);
+
+ /* The UST app session is held registry shall not be null */
assert(registry);
/* Push metadata for application before freeing the application. */
@@ -5320,7 +5338,7 @@ static int reply_ust_register_channel(int sock, int
sobjd, int cobjd,
/* Lookup application. If not found, there is a code flow error. */
app = find_app_by_notify_sock(sock);
if (!app) {
- DBG("Application socket %d is being teardown. Abort event
notify",
+ DBG("Application socket %d is being torn down. Abort event
notify",
sock);
ret = 0;
free(fields);
@@ -5330,7 +5348,7 @@ static int reply_ust_register_channel(int sock, int
sobjd, int cobjd,
/* Lookup channel by UST object descriptor. */
ua_chan = find_channel_by_objd(app, cobjd);
if (!ua_chan) {
- DBG("Application channel is being teardown. Abort event
notify");
+ DBG("Application channel is being torn down. Abort event
notify");
ret = 0;
free(fields);
goto error_rcu_unlock;
@@ -5341,7 +5359,12 @@ static int reply_ust_register_channel(int sock, int
sobjd, int cobjd,
/* Get right session registry depending on the session buffer type. */
registry = get_session_registry(ua_sess);
- assert(registry);
+ if (!registry) {
+ DBG("Application session is being torn down. Abort event
notify");
+ ret = 0;
+ free(fields);
+ goto error_rcu_unlock;
+ };
/* Depending on the buffer type, a different channel key is used. */
if (ua_sess->buffer_type == LTTNG_BUFFER_PER_UID) {
@@ -5439,7 +5462,7 @@ static int add_event_ust_registry(int sock, int sobjd,
int cobjd, char *name,
/* Lookup application. If not found, there is a code flow error. */
app = find_app_by_notify_sock(sock);
if (!app) {
- DBG("Application socket %d is being teardown. Abort event
notify",
+ DBG("Application socket %d is being torn down. Abort event
notify",
sock);
ret = 0;
free(sig);
@@ -5451,7 +5474,7 @@ static int add_event_ust_registry(int sock, int sobjd,
int cobjd, char *name,
/* Lookup channel by UST object descriptor. */
ua_chan = find_channel_by_objd(app, cobjd);
if (!ua_chan) {
- DBG("Application channel is being teardown. Abort event
notify");
+ DBG("Application channel is being torn down. Abort event
notify");
ret = 0;
free(sig);
free(fields);
@@ -5463,7 +5486,14 @@ static int add_event_ust_registry(int sock, int sobjd,
int cobjd, char *name,
ua_sess = ua_chan->session;
registry = get_session_registry(ua_sess);
- assert(registry);
+ if (!registry) {
+ DBG("Application session is being torn down. Abort event
notify");
+ ret = 0;
+ free(sig);
+ free(fields);
+ free(model_emf_uri);
+ goto error_rcu_unlock;
+ }
if (ua_sess->buffer_type == LTTNG_BUFFER_PER_UID) {
chan_reg_key = ua_chan->tracing_channel_id;
@@ -5551,7 +5581,11 @@ static int add_enum_ust_registry(int sock, int sobjd,
char *name,
}
registry = get_session_registry(ua_sess);
- assert(registry);
+ if (!registry) {
+ DBG("Application session is being torn down. Aborting enum
registration.");
+ free(entries);
+ goto error_rcu_unlock;
+ }
pthread_mutex_lock(®istry->lock);
@@ -5917,7 +5951,10 @@ int ust_app_snapshot_record(struct ltt_ust_session
*usess,
}
registry = get_session_registry(ua_sess);
- assert(registry);
+ if (!registry) {
+ DBG("Application session is being torn down.
Abort snapshot record.");
+ goto error;
+ }
ret = consumer_snapshot_channel(socket,
registry->metadata_key, output,
1, ua_sess->euid, ua_sess->egid,
pathname, wait, 0);
if (ret < 0) {
--
2.7.4
_______________________________________________
lttng-dev mailing list
lttng-dev@lists.lttng.org
https://lists.lttng.org/cgi-bin/mailman/listinfo/lttng-dev
