Fix: sessiond: crash enabling event rules that differ only by loglevel type

Issue observed
--------------

Summarizing bug #1373, an assertion fails when enabling two event-rules
that only differ by their log level selection type (all, range, or
single).

This issue can be reproduced by launching an instrumented
application (which remains active over the duration of this test) and
running:
  $ lttng create test_session
  $ lttng enable-channel --userspace test_channel
  $ lttng start test_session
  $ lttng enable-event --userspace --session test_session --channel test_channel 'l*' --loglevel-only=TRACE_DEBUG_LINE
  $ lttng enable-event --userspace --session test_session --channel test_channel 'l*' --loglevel=TRACE_DEBUG_LINE

Cause
-----

A number of sites conflate log level values and type. A clean-up had
been performed previously as of 2106efa08 and through follow-up commits.
However, some instances were apparently missed at the time.

As such, add_unique_ust_app_event mixed loglevel values and types when
initializing the key used for the unique insertion. The log level type,
for its part, is completely ignored.

Going back to the reproducer above, the first insertion succeeds as
expected. The second insertion fails since there is already an app event
rule with the `TRACE_DEBUG_LINE` log level type.

Moreover, the matching function only matches on the log level
type (which is really the value), which is also a bug.

The "matching" function is invoked on the key of the second event rule
and the first event rule since the hashing is only performed on the
event-rule's name pattern, resulting in a collision.

Solution
--------

Both the log level value and log level types are used to perform the
matching within the ust-app module. This implies extending the
ust_app_ht_key to store the log level value.

To simplify the matching logic (which attempted to handle 0 and -1
having the same meaning when the "ALL" log level type was used), the log
level value is normalized to '-1' throughout.

Fixes #1373
Reported-by: Bogdan Codres <bogdan.codres@windriver.com>
Change-Id: Id8090156646e4ef2011738623e0bfee1f400dced
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>

diff --git a/src/bin/lttng-sessiond/ust-app.c b/src/bin/lttng-sessiond/ust-app.c
index e5d6857a3ed4f5229e6db5199aa7ed90677a8973..163767d15cb8ef70101b9428f269b321c1c6ae57 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.c
+++ b/src/bin/lttng-sessiond/ust-app.c
@@ -116,14 +116,12 @@ static int ht_match_ust_app_event(struct cds_lfht_node *node, const void *_key)
 {
        struct ust_app_event *event;
        const struct ust_app_ht_key *key;
-       int ev_loglevel_value;
 
        assert(node);
        assert(_key);
 
        event = caa_container_of(node, struct ust_app_event, node.node);
        key = _key;
-       ev_loglevel_value = event->attr.loglevel;
 
        /* Match the 4 elements of the key: name, filter, loglevel, exclusions */
 
@@ -133,19 +131,10 @@ static int ht_match_ust_app_event(struct cds_lfht_node *node, const void *_key)
        }
 
        /* Event loglevel. */
-       if (ev_loglevel_value != key->loglevel_type) {
-               if (event->attr.loglevel_type == LTTNG_UST_ABI_LOGLEVEL_ALL
-                               && key->loglevel_type == 0 &&
-                               ev_loglevel_value == -1) {
-                       /*
-                        * Match is accepted. This is because on event creation, the
-                        * loglevel is set to -1 if the event loglevel type is ALL so 0 and
-                        * -1 are accepted for this loglevel type since 0 is the one set by
-                        * the API when receiving an enable event.
-                        */
-               } else {
-                       goto no_match;
-               }
+       if (!loglevels_match(event->attr.loglevel_type, event->attr.loglevel,
+                           key->loglevel_type, key->loglevel_value,
+                           LTTNG_UST_ABI_LOGLEVEL_ALL)) {
+               goto no_match;
        }
 
        /* One of the filters is NULL, fail. */
@@ -202,7 +191,9 @@ static void add_unique_ust_app_event(struct ust_app_channel *ua_chan,
        ht = ua_chan->events;
        key.name = event->attr.name;
        key.filter = event->filter;
-       key.loglevel_type = event->attr.loglevel;
+       key.loglevel_type = (enum lttng_ust_abi_loglevel_type)
+                                           event->attr.loglevel_type;
+       key.loglevel_value = event->attr.loglevel;
        key.exclusion = event->exclusion;
 
        node_ptr = cds_lfht_add_unique(ht->ht,
@@ -1499,7 +1490,9 @@ error:
  * Return an ust_app_event object or NULL on error.
  */
 static struct ust_app_event *find_ust_app_event(struct lttng_ht *ht,
-               const char *name, const struct lttng_bytecode *filter,
+               const char *name,
+               const struct lttng_bytecode *filter,
+               enum lttng_ust_abi_loglevel_type loglevel_type,
                int loglevel_value,
                const struct lttng_event_exclusion *exclusion)
 {
@@ -1514,7 +1507,8 @@ static struct ust_app_event *find_ust_app_event(struct lttng_ht *ht,
        /* Setup key for event lookup. */
        key.name = name;
        key.filter = filter;
-       key.loglevel_type = loglevel_value;
+       key.loglevel_type = loglevel_type;
+       key.loglevel_value = loglevel_value;
        /* lttng_event_exclusion and lttng_ust_event_exclusion structures are similar */
        key.exclusion = exclusion;
 
@@ -4841,9 +4835,11 @@ int ust_app_disable_event_glb(struct ltt_ust_session *usess,
                }
                ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
 
-               ua_event = find_ust_app_event(ua_chan->events, uevent->attr.name,
-                               uevent->filter, uevent->attr.loglevel,
-                               uevent->exclusion);
+               ua_event = find_ust_app_event(ua_chan->events,
+                               uevent->attr.name, uevent->filter,
+                               (enum lttng_ust_abi_loglevel_type)
+                                               uevent->attr.loglevel_type,
+                               uevent->attr.loglevel, uevent->exclusion);
                if (ua_event == NULL) {
                        DBG2("Event %s not found in channel %s for app pid %d."
                                        "Skipping", uevent->attr.name, uchan->name, app->pid);
@@ -5001,8 +4997,11 @@ int ust_app_enable_event_glb(struct ltt_ust_session *usess,
                ua_chan = caa_container_of(ua_chan_node, struct ust_app_channel, node);
 
                /* Get event node */
-               ua_event = find_ust_app_event(ua_chan->events, uevent->attr.name,
-                               uevent->filter, uevent->attr.loglevel, uevent->exclusion);
+               ua_event = find_ust_app_event(ua_chan->events,
+                               uevent->attr.name, uevent->filter,
+                               (enum lttng_ust_abi_loglevel_type)
+                                               uevent->attr.loglevel_type,
+                               uevent->attr.loglevel, uevent->exclusion);
                if (ua_event == NULL) {
                        DBG3("UST app enable event %s not found for app PID %d."
                                        "Skipping app", uevent->attr.name, app->pid);
@@ -5776,7 +5775,10 @@ int ust_app_channel_synchronize_event(struct ust_app_channel *ua_chan,
        struct ust_app_event *ua_event = NULL;
 
        ua_event = find_ust_app_event(ua_chan->events, uevent->attr.name,
-               uevent->filter, uevent->attr.loglevel, uevent->exclusion);
+                       uevent->filter,
+                       (enum lttng_ust_abi_loglevel_type)
+                                       uevent->attr.loglevel_type,
+                       uevent->attr.loglevel, uevent->exclusion);
        if (!ua_event) {
                ret = create_ust_app_event(ua_sess, ua_chan, uevent, app);
                if (ret < 0) {

diff --git a/src/bin/lttng-sessiond/ust-app.h b/src/bin/lttng-sessiond/ust-app.h
index 4a7a7c92dfe72e62d312652d596302689ca63003..568bd09e478048af0720cd5e877fe1f478d8ee7d 100644 (file)
--- a/src/bin/lttng-sessiond/ust-app.h
+++ b/src/bin/lttng-sessiond/ust-app.h
@@ -42,6 +42,7 @@ struct ust_app_ht_key {
        const char *name;
        const struct lttng_bytecode *filter;
        enum lttng_ust_abi_loglevel_type loglevel_type;
+       int loglevel_value;
        const struct lttng_event_exclusion *exclusion;
 };
 

diff --git a/src/common/event-rule/jul-logging.c b/src/common/event-rule/jul-logging.c
index 5f69f84c723b0636c48d844c4a920c2e7629afe3..ddaa7ba696f67b2070fd6dd4b22d77d5cfe9c6ac 100644 (file)
--- a/src/common/event-rule/jul-logging.c
+++ b/src/common/event-rule/jul-logging.c
@@ -442,7 +442,7 @@ static struct lttng_event *lttng_event_rule_jul_logging_generate_lttng_event(
                        rule, &log_level_rule);
        if (status == LTTNG_EVENT_RULE_STATUS_UNSET) {
                loglevel_type = LTTNG_EVENT_LOGLEVEL_ALL;
-               loglevel_value = 0;
+               loglevel_value = -1;
        } else if (status == LTTNG_EVENT_RULE_STATUS_OK) {
                enum lttng_log_level_rule_status llr_status;
 

diff --git a/src/common/event-rule/log4j-logging.c b/src/common/event-rule/log4j-logging.c
index 755ae26c626c4afc045b603d2f37bf357ee68389..b4d67667dffc8a170e184e183a1f1d89a9c6e588 100644 (file)
--- a/src/common/event-rule/log4j-logging.c
+++ b/src/common/event-rule/log4j-logging.c
@@ -442,7 +442,7 @@ static struct lttng_event *lttng_event_rule_log4j_logging_generate_lttng_event(
                        rule, &log_level_rule);
        if (status == LTTNG_EVENT_RULE_STATUS_UNSET) {
                loglevel_type = LTTNG_EVENT_LOGLEVEL_ALL;
-               loglevel_value = 0;
+               loglevel_value = -1;
        } else if (status == LTTNG_EVENT_RULE_STATUS_OK) {
                enum lttng_log_level_rule_status llr_status;
 

diff --git a/src/common/event-rule/python-logging.c b/src/common/event-rule/python-logging.c
index 4b5cebaf02f6e4a1cceac1022db64c9f6847c1d8..171de4647bb45ee4184fc522a7abe4bae8befaf7 100644 (file)
--- a/src/common/event-rule/python-logging.c
+++ b/src/common/event-rule/python-logging.c
@@ -442,7 +442,7 @@ static struct lttng_event *lttng_event_rule_python_logging_generate_lttng_event(
                        rule, &log_level_rule);
        if (status == LTTNG_EVENT_RULE_STATUS_UNSET) {
                loglevel_type = LTTNG_EVENT_LOGLEVEL_ALL;
-               loglevel_value = 0;
+               loglevel_value = -1;
        } else if (status == LTTNG_EVENT_RULE_STATUS_OK) {
                enum lttng_log_level_rule_status llr_status;