From e0c7ec2b3d96dd7ddd9375c5f72239a733d0cf24 Mon Sep 17 00:00:00 2001 From: David Goulet Date: Mon, 23 Jan 2012 12:01:35 -0500 Subject: [PATCH] Add UST version validation Upon registration of an UST app, we now check the version compatibility with the session daemon and set a compatible bit. Only the major version is validated since major version bump implies ABI/API breakage. For now, every action on the application list check the app compatible bit before executing it. We are too close from a stable release so hard *not standard* check is done across the map for ust-app. The downside is that the client will not get informed of any incompatible app upon any action on the tracer using lttng cli. If the version is NOT compatible, the register done is sent and the app is kept in the registry. We can't close the socket or deny the registration because if the env. var LTTNG_UST_REGISTER_TIMEOUT=-1 is set on the app side, infinite loop will occur and possibly stall the app. New sessiond comm. version error added but not use at this commit. Signed-off-by: David Goulet --- src/bin/lttng-sessiond/main.c | 12 +- src/bin/lttng-sessiond/ust-app.c | 144 ++++++++++++++++++++++- src/bin/lttng-sessiond/ust-app.h | 13 ++ src/common/sessiond-comm/sessiond-comm.c | 1 + src/common/sessiond-comm/sessiond-comm.h | 1 + 5 files changed, 164 insertions(+), 7 deletions(-) diff --git a/src/bin/lttng-sessiond/main.c b/src/bin/lttng-sessiond/main.c index e1d50974b..886456a0c 100644 --- a/src/bin/lttng-sessiond/main.c +++ b/src/bin/lttng-sessiond/main.c @@ -1110,10 +1110,16 @@ static void *thread_manage_apps(void *data) } /* - * Add channel(s) and event(s) to newly registered apps - * from lttng global UST domain. + * Validate UST version compatibility. */ - update_ust_app(ust_cmd.sock); + ret = ust_app_validate_version(ust_cmd.sock); + if (ret >= 0) { + /* + * Add channel(s) and event(s) to newly registered apps + * from lttng global UST domain. + */ + update_ust_app(ust_cmd.sock); + } ret = ust_app_register_done(ust_cmd.sock); if (ret < 0) { diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c index e2b32a51a..62816cecb 100644 --- a/src/bin/lttng-sessiond/ust-app.c +++ b/src/bin/lttng-sessiond/ust-app.c @@ -1284,6 +1284,7 @@ int ust_app_register(struct ust_register_msg *msg, int sock) lta->ppid = msg->ppid; lta->uid = msg->uid; lta->gid = msg->gid; + lta->compatible = 0; /* Not compatible until proven */ lta->bits_per_long = msg->bits_per_long; lta->v_major = msg->major; lta->v_minor = msg->minor; @@ -1391,6 +1392,13 @@ int ust_app_list_events(struct lttng_event **events) cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { struct lttng_ust_tracepoint_iter uiter; + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } handle = ustctl_tracepoint_list(app->key.sock); if (handle < 0) { ERR("UST app list events getting handle failed for app pid %d", @@ -1499,6 +1507,13 @@ int ust_app_disable_channel_glb(struct ltt_ust_session *usess, /* For every registered applications */ cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { struct lttng_ht_iter uiter; + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); if (ua_sess == NULL) { continue; @@ -1552,6 +1567,13 @@ int ust_app_enable_channel_glb(struct ltt_ust_session *usess, /* For every registered applications */ cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); if (ua_sess == NULL) { continue; @@ -1592,6 +1614,13 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess, /* For all registered applications */ cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); if (ua_sess == NULL) { /* Next app */ @@ -1651,6 +1680,13 @@ int ust_app_disable_all_event_glb(struct ltt_ust_session *usess, /* For all registered applications */ cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); /* If ua_sess is NULL, there is a code flow error */ assert(ua_sess); @@ -1701,6 +1737,13 @@ int ust_app_create_channel_glb(struct ltt_ust_session *usess, /* For every registered applications */ cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } /* * Create session on the tracer side and add it to app session HT. Note * that if session exist, it will simply return a pointer to the ust @@ -1755,6 +1798,13 @@ int ust_app_enable_event_glb(struct ltt_ust_session *usess, /* For all registered applications */ cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); /* If ua_sess is NULL, there is a code flow error */ assert(ua_sess); @@ -1808,6 +1858,13 @@ int ust_app_create_event_glb(struct ltt_ust_session *usess, /* For all registered applications */ cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); /* If ua_sess is NULL, there is a code flow error */ assert(ua_sess); @@ -1853,6 +1910,10 @@ int ust_app_start_trace(struct ltt_ust_session *usess, struct ust_app *app) rcu_read_lock(); + if (!app->compatible) { + goto end; + } + ua_sess = lookup_session_by_app(usess, app); if (ua_sess == NULL) { goto error_rcu_unlock; @@ -1929,11 +1990,11 @@ skip_setup: goto error_rcu_unlock; } - rcu_read_unlock(); - /* Quiescent wait after starting trace */ ustctl_wait_quiescent(app->key.sock); +end: + rcu_read_unlock(); return 0; error_rcu_unlock: @@ -1955,6 +2016,10 @@ int ust_app_stop_trace(struct ltt_ust_session *usess, struct ust_app *app) rcu_read_lock(); + if (!app->compatible) { + goto end; + } + ua_sess = lookup_session_by_app(usess, app); if (ua_sess == NULL) { /* Only malloc can failed so something is really wrong */ @@ -2021,6 +2086,10 @@ int ust_app_destroy_trace(struct ltt_ust_session *usess, struct ust_app *app) rcu_read_lock(); + if (!app->compatible) { + goto end; + } + __lookup_session_by_app(usess, app, &iter); node = lttng_ht_iter_get_node_ulong(&iter); if (node == NULL) { @@ -2038,11 +2107,11 @@ int ust_app_destroy_trace(struct ltt_ust_session *usess, struct ust_app *app) delete_ust_app_session(app->key.sock, ua_sess); - rcu_read_unlock(); - /* Quiescent wait after stopping trace */ ustctl_wait_quiescent(app->key.sock); +end: + rcu_read_unlock(); return 0; error_rcu_unlock: @@ -2156,6 +2225,10 @@ void ust_app_global_update(struct ltt_ust_session *usess, int sock) goto error; } + if (!app->compatible) { + goto error; + } + ua_sess = create_ust_app_session(usess, app); if (ua_sess == NULL) { goto error; @@ -2215,6 +2288,13 @@ int ust_app_add_ctx_channel_glb(struct ltt_ust_session *usess, rcu_read_lock(); cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); if (ua_sess == NULL) { continue; @@ -2257,6 +2337,13 @@ int ust_app_add_ctx_event_glb(struct ltt_ust_session *usess, rcu_read_lock(); cds_lfht_for_each_entry(ust_app_ht->ht, &iter.iter, app, node.node) { + if (!app->compatible) { + /* + * TODO: In time, we should notice the caller of this error by + * telling him that this is a version error. + */ + continue; + } ua_sess = lookup_session_by_app(usess, app); if (ua_sess == NULL) { continue; @@ -2314,6 +2401,11 @@ int ust_app_enable_event_pid(struct ltt_ust_session *usess, goto error; } + if (!app->compatible) { + ret = 0; + goto error; + } + ua_sess = lookup_session_by_app(usess, app); /* If ua_sess is NULL, there is a code flow error */ assert(ua_sess); @@ -2372,6 +2464,11 @@ int ust_app_disable_event_pid(struct ltt_ust_session *usess, goto error; } + if (!app->compatible) { + ret = 0; + goto error; + } + ua_sess = lookup_session_by_app(usess, app); /* If ua_sess is NULL, there is a code flow error */ assert(ua_sess); @@ -2402,3 +2499,42 @@ error: rcu_read_unlock(); return ret; } + +/* + * Validate version of UST apps and set the compatible bit. + */ +int ust_app_validate_version(int sock) +{ + int ret; + struct ust_app *app; + + rcu_read_lock(); + + app = find_app_by_sock(sock); + assert(app); + + ret = ustctl_tracer_version(sock, &app->version); + if (ret < 0) { + goto error; + } + + /* Validate version */ + if (app->version.major > UST_APP_MAJOR_VERSION) { + goto error; + } + + DBG2("UST app PID %d is compatible with major version %d " + "(supporting <= %d)", app->key.pid, app->version.major, + UST_APP_MAJOR_VERSION); + app->compatible = 1; + rcu_read_unlock(); + return 0; + +error: + DBG2("UST app PID %d is not compatible with major version %d " + "(supporting <= %d)", app->key.pid, app->version.major, + UST_APP_MAJOR_VERSION); + app->compatible = 0; + rcu_read_unlock(); + return -1; +} diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h index d7fbcd5d0..67c09692f 100644 --- a/src/bin/lttng-sessiond/ust-app.h +++ b/src/bin/lttng-sessiond/ust-app.h @@ -23,6 +23,9 @@ #include "trace-ust.h" +/* lttng-ust supported version. */ +#define UST_APP_MAJOR_VERSION 1 + #define UST_APP_EVENT_LIST_SIZE 32 extern int ust_consumerd64_fd, ust_consumerd32_fd; @@ -107,6 +110,10 @@ struct ust_app { uid_t uid; /* User ID that owns the apps */ gid_t gid; /* Group ID that owns the apps */ int bits_per_long; + int compatible; /* If the lttng-ust tracer version does not match the + supported version of the session daemon, this flag is + set to 0 (NOT compatible) else 1. */ + struct lttng_ust_tracer_version version; uint32_t v_major; /* Verion major number */ uint32_t v_minor; /* Verion minor number */ char name[17]; /* Process name (short) */ @@ -165,6 +172,7 @@ void ust_app_clean_list(void); void ust_app_ht_alloc(void); struct lttng_ht *ust_app_get_ht(void); struct ust_app *ust_app_find_by_pid(pid_t pid); +int ust_app_validate_version(int sock); #else /* HAVE_LIBLTTNG_UST_CTL */ @@ -320,6 +328,11 @@ int ust_app_disable_event_pid(struct ltt_ust_session *usess, { return 0; } +static inline +int ust_app_validate_version(int sock) +{ + return 0; +} #endif /* HAVE_LIBLTTNG_UST_CTL */ diff --git a/src/common/sessiond-comm/sessiond-comm.c b/src/common/sessiond-comm/sessiond-comm.c index b005cc713..03e89314f 100644 --- a/src/common/sessiond-comm/sessiond-comm.c +++ b/src/common/sessiond-comm/sessiond-comm.c @@ -76,6 +76,7 @@ static const char *lttcomm_readable_code[] = { [ LTTCOMM_ERR_INDEX(LTTCOMM_KERN_DIR_EXIST) ] = "Kernel trace directory already exist", [ LTTCOMM_ERR_INDEX(LTTCOMM_KERN_NO_SESSION) ] = "No kernel session found", [ LTTCOMM_ERR_INDEX(LTTCOMM_KERN_LIST_FAIL) ] = "Listing kernel events failed", + [ LTTCOMM_ERR_INDEX(LTTCOMM_UST_VERSION) ] = "UST tracer version is not compatible", [ LTTCOMM_ERR_INDEX(LTTCOMM_UST_SESS_FAIL) ] = "UST create session failed", [ LTTCOMM_ERR_INDEX(LTTCOMM_UST_CHAN_FAIL) ] = "UST create channel failed", [ LTTCOMM_ERR_INDEX(LTTCOMM_UST_CHAN_EXIST) ] = "UST channel already exist", diff --git a/src/common/sessiond-comm/sessiond-comm.h b/src/common/sessiond-comm/sessiond-comm.h index 2e9279555..694ee2e4e 100644 --- a/src/common/sessiond-comm/sessiond-comm.h +++ b/src/common/sessiond-comm/sessiond-comm.h @@ -111,6 +111,7 @@ enum lttcomm_return_code { LTTCOMM_KERN_DIR_EXIST, /* Kernel trace directory exist */ LTTCOMM_KERN_NO_SESSION, /* No kernel session found */ LTTCOMM_KERN_LIST_FAIL, /* Kernel listing events failed */ + LTTCOMM_UST_VERSION, /* UST tracer version is not compatible */ LTTCOMM_UST_SESS_FAIL, /* UST create session failed */ LTTCOMM_UST_CHAN_EXIST, /* UST channel already exist */ LTTCOMM_UST_CHAN_FAIL, /* UST create channel failed */ -- 2.34.1