Fix: racy notifier captures update vs traversal

For captures, a new struct lttng_kernel_notification_ctx is introduced,
which is to be used as additional "context" to the notification_send()
callback. This allows passing the "eval_capture" state from the probe
to the notification callback, and fixes a bug where a sequence of:

- create notification enabler,
- enable notification enabler,
- add capture to enabler,

where a tracepoint runs concurrently with add capture happens to do a
first capture list_empty check which skips the stack preparation,
whereas the second capture list_empty check within the notification
callback finds a capture entry, and thus attempts to use an
uninitialized stack. The notification callback is also modified to use
an RCU-aware list traversal.

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

diff --git a/include/lttng/event-notifier-notification.h b/include/lttng/event-notifier-notification.h
index b044e2fa86f89f777ff968ccceb340d8fb255e4c..160b25429e7830625a910719d917a2c4349dc432 100644 (file)
--- a/include/lttng/event-notifier-notification.h
+++ b/include/lttng/event-notifier-notification.h
@@ -12,6 +12,7 @@
 
 void lttng_event_notifier_notification_send(struct lttng_event_notifier *event_notifier,
                struct lttng_probe_ctx *lttng_probe_ctx,
-               const char *stack_data);
+               const char *stack_data,
+               struct lttng_kernel_notifier_ctx *notif_ctx);
 
 #endif /* _LTTNG_EVENT_NOTIFIER_NOTIFICATION_H */

diff --git a/include/lttng/events.h b/include/lttng/events.h
index 6aa880f82eb1b8737dad767b0c5e94a23a159888..5d439bcbbcc0a1dc8321819032edb860565c867c 100644 (file)
--- a/include/lttng/events.h
+++ b/include/lttng/events.h
@@ -353,6 +353,10 @@ struct lttng_event {
        int has_enablers_without_bytecode;
 };
 
+struct lttng_kernel_notifier_ctx {
+       int eval_capture;
+};
+
 // FIXME: Really similar to lttng_event above. Could those be merged ?
 struct lttng_event_notifier {
        enum lttng_event_type evtype;   /* First field. */
@@ -385,10 +389,12 @@ struct lttng_event_notifier {
        size_t num_captures;
        struct list_head capture_bytecode_runtime_head;
        int has_enablers_without_bytecode;
+       int eval_capture;               /* Should evaluate capture */
 
        void (*send_notification)(struct lttng_event_notifier *event_notifier,
                        struct lttng_probe_ctx *lttng_probe_ctx,
-                       const char *interpreter_stack_data);
+                       const char *interpreter_stack_data,
+                       struct lttng_kernel_notifier_ctx *notif_ctx);
        struct lttng_event_notifier_group *group; /* Weak ref */
 };
 

diff --git a/include/lttng/tracepoint-event-impl.h b/include/lttng/tracepoint-event-impl.h
index 079d7d37006e9e677f1ae5d16d33e680378993f7..f5464773cd47fb1e0059ccda2df9ccc6c33feaed 100644 (file)
--- a/include/lttng/tracepoint-event-impl.h
+++ b/include/lttng/tracepoint-event-impl.h
@@ -1430,6 +1430,8 @@ static void __event_notifier_probe__##_name(void *__data, _proto)       \
        struct probe_local_vars __tp_locvar;                                  \
        struct probe_local_vars *tp_locvar __attribute__((unused)) =          \
                        &__tp_locvar;                                         \
+       struct lttng_kernel_notifier_ctx __notif_ctx;                         \
+       bool __interpreter_stack_prepared = false;                            \
                                                                              \
        if (unlikely(!READ_ONCE(__event_notifier->enabled)))                  \
                return;                                                       \
@@ -1440,6 +1442,7 @@ static void __event_notifier_probe__##_name(void *__data, _proto)       \
                                                                              \
                __event_prepare_interpreter_stack__##_name(__stackvar.__interpreter_stack_data, \
                                tp_locvar, _args);                            \
+               __interpreter_stack_prepared = true;                          \
                lttng_list_for_each_entry_rcu(bc_runtime, &__event_notifier->filter_bytecode_runtime_head, node) { \
                        if (unlikely(bc_runtime->interpreter_funcs.filter(bc_runtime, &__lttng_probe_ctx,       \
                                        __stackvar.__interpreter_stack_data) & LTTNG_INTERPRETER_RECORD_FLAG))  \
@@ -1449,14 +1452,16 @@ static void __event_notifier_probe__##_name(void *__data, _proto)             \
                        goto __post;                                          \
        }                                                                     \
                                                                              \
-       if (unlikely(!list_empty(&__event_notifier->capture_bytecode_runtime_head)))    \
+       __notif_ctx.eval_capture = LTTNG_READ_ONCE(__event_notifier->eval_capture); \
+       if (unlikely(!__interpreter_stack_prepared && __notif_ctx.eval_capture)) \
                __event_prepare_interpreter_stack__##_name(                   \
                                __stackvar.__interpreter_stack_data,          \
                                tp_locvar, _args);                            \
                                                                              \
        __event_notifier->send_notification(__event_notifier,                 \
                        &__lttng_probe_ctx,                                   \
-                       __stackvar.__interpreter_stack_data);                 \
+                       __stackvar.__interpreter_stack_data,                  \
+                       &__notif_ctx);                                        \
                                                                              \
 __post:                                                                              \
        _code_post                                                            \
@@ -1481,6 +1486,8 @@ static void __event_notifier_probe__##_name(void *__data)               \
        struct probe_local_vars __tp_locvar;                                  \
        struct probe_local_vars *tp_locvar __attribute__((unused)) =          \
                        &__tp_locvar;                                         \
+       struct lttng_kernel_notifier_ctx __notif_ctx;                         \
+       bool __interpreter_stack_prepared = false;                            \
                                                                              \
        if (unlikely(!READ_ONCE(__event_notifier->enabled)))                  \
                return;                                                       \
@@ -1491,6 +1498,7 @@ static void __event_notifier_probe__##_name(void *__data)               \
                                                                              \
                __event_prepare_interpreter_stack__##_name(__stackvar.__interpreter_stack_data, \
                                tp_locvar);                                   \
+               __interpreter_stack_prepared = true;                          \
                lttng_list_for_each_entry_rcu(bc_runtime, &__event_notifier->filter_bytecode_runtime_head, node) { \
                        if (unlikely(bc_runtime->interpreter_funcs.filter(bc_runtime, &__lttng_probe_ctx,       \
                                        __stackvar.__interpreter_stack_data) & LTTNG_INTERPRETER_RECORD_FLAG))  \
@@ -1500,14 +1508,16 @@ static void __event_notifier_probe__##_name(void *__data)                     \
                        goto __post;                                          \
        }                                                                     \
                                                                              \
-       if (unlikely(!list_empty(&__event_notifier->capture_bytecode_runtime_head)))  \
+       __notif_ctx.eval_capture = LTTNG_READ_ONCE(__event_notifier->eval_capture); \
+       if (unlikely(!__interpreter_stack_prepared && __notif_ctx.eval_capture)) \
                __event_prepare_interpreter_stack__##_name(                   \
                                __stackvar.__interpreter_stack_data,          \
                                tp_locvar);                                   \
                                                                              \
        __event_notifier->send_notification(__event_notifier,                 \
                        &__lttng_probe_ctx,                                   \
-                       __stackvar.__interpreter_stack_data);                 \
+                       __stackvar.__interpreter_stack_data,                  \
+                       &__notif_ctx);                                        \
 __post:                                                                              \
        _code_post                                                            \
        return;                                                               \

diff --git a/src/lttng-event-notifier-notification.c b/src/lttng-event-notifier-notification.c
index 52b5593ac9325b5324c9169f72ff736044a49cac..b39bfaad58545429beaf217badc56b81df7da3dc 100644 (file)
--- a/src/lttng-event-notifier-notification.c
+++ b/src/lttng-event-notifier-notification.c
@@ -426,7 +426,8 @@ void notification_send(struct lttng_event_notifier_notification *notif,
 
 void lttng_event_notifier_notification_send(struct lttng_event_notifier *event_notifier,
                struct lttng_probe_ctx *lttng_probe_ctx,
-               const char *stack_data)
+               const char *stack_data,
+               struct lttng_kernel_notifier_ctx *notif_ctx)
 {
        struct lttng_event_notifier_notification notif = { 0 };
        int ret;
@@ -440,7 +441,7 @@ void lttng_event_notifier_notification_send(struct lttng_event_notifier *event_n
                goto end;
        }
 
-       if (unlikely(!list_empty(&event_notifier->capture_bytecode_runtime_head))) {
+       if (unlikely(notif_ctx->eval_capture)) {
                struct lttng_bytecode_runtime *capture_bc_runtime;
 
                /*
@@ -449,7 +450,7 @@ void lttng_event_notifier_notification_send(struct lttng_event_notifier *event_n
                 * `output` parameter to the capture buffer. If the interpreter
                 * fails, append an empty capture to the buffer.
                 */
-               list_for_each_entry(capture_bc_runtime,
+               list_for_each_entry_rcu(capture_bc_runtime,
                                &event_notifier->capture_bytecode_runtime_head, node) {
                        struct lttng_interpreter_output output;
 

diff --git a/src/lttng-events.c b/src/lttng-events.c
index d819c9e20dac0e28719b61aeb6373da868cc5efc..a144f544c52540170ea0e9478c263780d05b856d 100644 (file)
--- a/src/lttng-events.c
+++ b/src/lttng-events.c
@@ -2768,6 +2768,8 @@ void lttng_event_notifier_group_sync_enablers(struct lttng_event_notifier_group
                list_for_each_entry(runtime,
                                &event_notifier->capture_bytecode_runtime_head, node)
                        lttng_bytecode_capture_sync_state(runtime);
+
+               WRITE_ONCE(event_notifier->eval_capture, !!event_notifier->num_captures);
        }
 }
 

diff --git a/src/probes/lttng-kprobes.c b/src/probes/lttng-kprobes.c
index 6824088c3d752d9f38b754894fae3cab932d6d9a..dc18f5077fc1091afab28119dc352deccc09287d 100644 (file)
--- a/src/probes/lttng-kprobes.c
+++ b/src/probes/lttng-kprobes.c
@@ -54,11 +54,13 @@ int lttng_kprobes_event_notifier_handler_pre(struct kprobe *p, struct pt_regs *r
 {
        struct lttng_event_notifier *event_notifier =
                container_of(p, struct lttng_event_notifier, u.kprobe.kp);
+       struct lttng_kernel_notifier_ctx notif_ctx;
 
        if (unlikely(!READ_ONCE(event_notifier->enabled)))
                return 0;
 
-       event_notifier->send_notification(event_notifier, NULL, NULL);
+       notif_ctx.eval_capture = LTTNG_READ_ONCE(event_notifier->eval_capture);
+       event_notifier->send_notification(event_notifier, NULL, NULL, &notif_ctx);
 
        return 0;
 }

diff --git a/src/probes/lttng-uprobes.c b/src/probes/lttng-uprobes.c
index 031bfa7cd3affafed75130840405d9c08ad17438..bab941abe348d2c1d96a91cf118948ac0bb9c87d 100644 (file)
--- a/src/probes/lttng-uprobes.c
+++ b/src/probes/lttng-uprobes.c
@@ -69,11 +69,13 @@ int lttng_uprobes_event_notifier_handler_pre(struct uprobe_consumer *uc, struct
        struct lttng_uprobe_handler *uprobe_handler =
                container_of(uc, struct lttng_uprobe_handler, up_consumer);
        struct lttng_event_notifier *event_notifier = uprobe_handler->u.event_notifier;
+       struct lttng_kernel_notifier_ctx notif_ctx;
 
        if (unlikely(!READ_ONCE(event_notifier->enabled)))
                return 0;
 
-       event_notifier->send_notification(event_notifier, NULL, NULL);
+       notif_ctx.eval_capture = LTTNG_READ_ONCE(event_notifier->eval_capture);
+       event_notifier->send_notification(event_notifier, NULL, NULL, &notif_ctx);
        return 0;
 }