Fix: event notification capture error handling

When the captured fields end up taking more than

  PIPE_BUF - sizeof(struct lttng_ust_abi_event_notifier_notification) - 1

bytes of space for the msgpack message, the notification append capture
fails.

Currently, the result is that the msgpack buffer will contain a (likely
corrupted) truncated msgpack data.

Handle those overflow errors, and when they are encountered, reset the
msgpack writer position to skip the problematic captured field entirely.

Change-Id: I7ba1bf06aa72512fc73211a1d8ae6823d0e8d7ff

diff --git a/src/common/msgpack/msgpack.c b/src/common/msgpack/msgpack.c
index 10b06adc5b006335798a9ce28e2bd83679235054..d08569a8bc79a148048c65e2653b590747a0c839 100644 (file)
--- a/src/common/msgpack/msgpack.c
+++ b/src/common/msgpack/msgpack.c
@@ -483,6 +483,18 @@ int lttng_msgpack_write_double(struct lttng_msgpack_writer *writer, double value
        return lttng_msgpack_encode_f64(writer, value);
 }
 
+int lttng_msgpack_save_writer_pos(struct lttng_msgpack_writer *writer, uint8_t **pos)
+{
+       *pos = writer->write_pos;
+       return 0;
+}
+
+int lttng_msgpack_restore_writer_pos(struct lttng_msgpack_writer *writer, uint8_t *pos)
+{
+       writer->write_pos = pos;
+       return 0;
+}
+
 void lttng_msgpack_writer_init(struct lttng_msgpack_writer *writer,
                uint8_t *buffer, size_t size)
 {

diff --git a/src/common/msgpack/msgpack.h b/src/common/msgpack/msgpack.h
index 140ad38708b57a50c5736c061cb5700ce17b024f..d9933cf8136ecac419b96527b096ae28881011e7 100644 (file)
--- a/src/common/msgpack/msgpack.h
+++ b/src/common/msgpack/msgpack.h
@@ -67,4 +67,10 @@ int lttng_msgpack_begin_array(
 int lttng_msgpack_end_array(struct lttng_msgpack_writer *writer)
        __attribute__((visibility("hidden")));
 
+int lttng_msgpack_save_writer_pos(struct lttng_msgpack_writer *writer, uint8_t **pos)
+       __attribute__((visibility("hidden")));
+
+int lttng_msgpack_restore_writer_pos(struct lttng_msgpack_writer *writer, uint8_t *pos)
+       __attribute__((visibility("hidden")));
+
 #endif /* _LTTNG_UST_MSGPACK_H */

diff --git a/src/lib/lttng-ust/event-notifier-notification.c b/src/lib/lttng-ust/event-notifier-notification.c
index c13279df58d13351a8832d5b47b6447a074b3bfe..90b5bac24820b585402d993399bc0f0ac65ad2d8 100644 (file)
--- a/src/lib/lttng-ust/event-notifier-notification.c
+++ b/src/lib/lttng-ust/event-notifier-notification.c
@@ -39,27 +39,50 @@ struct lttng_event_notifier_notification {
 };
 
 static
-void capture_enum(struct lttng_msgpack_writer *writer,
+int capture_enum(struct lttng_msgpack_writer *writer,
                struct lttng_interpreter_output *output)
 {
-       lttng_msgpack_begin_map(writer, 2);
-       lttng_msgpack_write_str(writer, "type");
-       lttng_msgpack_write_str(writer, "enum");
+       int ret;
 
-       lttng_msgpack_write_str(writer, "value");
+       ret = lttng_msgpack_begin_map(writer, 2);
+       if (ret) {
+               goto end;
+       }
+       ret = lttng_msgpack_write_str(writer, "type");
+       if (ret) {
+               goto end;
+       }
+       ret = lttng_msgpack_write_str(writer, "enum");
+       if (ret) {
+               goto end;
+       }
+       ret = lttng_msgpack_write_str(writer, "value");
+       if (ret) {
+               goto end;
+       }
 
        switch (output->type) {
        case LTTNG_INTERPRETER_TYPE_SIGNED_ENUM:
-               lttng_msgpack_write_signed_integer(writer, output->u.s);
+               ret = lttng_msgpack_write_signed_integer(writer, output->u.s);
+               if (ret) {
+                       goto end;
+               }
                break;
        case LTTNG_INTERPRETER_TYPE_UNSIGNED_ENUM:
-               lttng_msgpack_write_signed_integer(writer, output->u.u);
+               ret = lttng_msgpack_write_signed_integer(writer, output->u.u);
+               if (ret) {
+                       goto end;
+               }
                break;
        default:
-               abort();
+               CRIT("Unknown enum output type\n");
+               ret = -1;
+               goto end;
        }
 
-       lttng_msgpack_end_map(writer);
+       ret = lttng_msgpack_end_map(writer);
+end:
+       return ret;
 }
 
 static
@@ -105,7 +128,8 @@ int64_t capture_sequence_element_signed(uint8_t *ptr,
                break;
        }
        default:
-               abort();
+               CRIT("Unknown sequence element size\n");
+               value = 0;
        }
 
        return value;
@@ -154,24 +178,27 @@ uint64_t capture_sequence_element_unsigned(uint8_t *ptr,
                break;
        }
        default:
-               abort();
+               CRIT("Unknown sequence element size\n");
+               value = 0;
        }
 
        return value;
 }
 
 static
-void capture_sequence(struct lttng_msgpack_writer *writer,
+int capture_sequence(struct lttng_msgpack_writer *writer,
                struct lttng_interpreter_output *output)
 {
        const struct lttng_ust_type_integer *integer_type;
        const struct lttng_ust_type_common *nested_type;
        uint8_t *ptr;
        bool signedness;
-       int i;
-
-       lttng_msgpack_begin_array(writer, output->u.sequence.nr_elem);
+       int i, ret;
 
+       ret = lttng_msgpack_begin_array(writer, output->u.sequence.nr_elem);
+       if (ret) {
+               goto end;
+       }
        ptr = (uint8_t *) output->u.sequence.ptr;
        nested_type = output->u.sequence.nested_type;
        switch (nested_type->type) {
@@ -183,18 +210,22 @@ void capture_sequence(struct lttng_msgpack_writer *writer,
                integer_type = lttng_ust_get_type_integer(lttng_ust_get_type_enum(nested_type)->container_type);
                break;
        default:
-               /* Capture of array of non-integer are not supported. */
-               abort();
+               CRIT("Capture of array of non-integer are not supported\n");
+               ret = -1;
+               goto end;
        }
        signedness = integer_type->signedness;
        for (i = 0; i < output->u.sequence.nr_elem; i++) {
                if (signedness) {
-                       lttng_msgpack_write_signed_integer(writer,
+                       ret = lttng_msgpack_write_signed_integer(writer,
                                capture_sequence_element_signed(ptr, integer_type));
                } else {
-                       lttng_msgpack_write_unsigned_integer(writer,
+                       ret = lttng_msgpack_write_unsigned_integer(writer,
                                capture_sequence_element_unsigned(ptr, integer_type));
                }
+               if (ret) {
+                       goto end;
+               }
 
                /*
                 * We assume that alignment is smaller or equal to the size.
@@ -209,14 +240,17 @@ void capture_sequence(struct lttng_msgpack_writer *writer,
                ptr += (integer_type->size / CHAR_BIT) ;
        }
 
-       lttng_msgpack_end_array(writer);
+       ret = lttng_msgpack_end_array(writer);
+end:
+       return ret;
 }
 
 static
-void notification_init(struct lttng_event_notifier_notification *notif,
+int notification_init(struct lttng_event_notifier_notification *notif,
                const struct lttng_ust_event_notifier *event_notifier)
 {
        struct lttng_msgpack_writer *writer = &notif->writer;
+       int ret = 0;
 
        notif->event_notifier_token = event_notifier->priv->parent.user_token;
        notif->notification_fd = event_notifier->priv->group->notification_fd;
@@ -226,48 +260,56 @@ void notification_init(struct lttng_event_notifier_notification *notif,
                lttng_msgpack_writer_init(writer, notif->capture_buf,
                                CAPTURE_BUFFER_SIZE);
 
-               lttng_msgpack_begin_array(writer, event_notifier->priv->num_captures);
+               ret = lttng_msgpack_begin_array(writer, event_notifier->priv->num_captures);
+               if (ret) {
+                       goto end;
+               }
                notif->has_captures = true;
        }
+end:
+       return ret;
 }
 
 static
-void notification_append_capture(
+int notification_append_capture(
                struct lttng_event_notifier_notification *notif,
                struct lttng_interpreter_output *output)
 {
        struct lttng_msgpack_writer *writer = &notif->writer;
+       int ret;
 
        switch (output->type) {
        case LTTNG_INTERPRETER_TYPE_S64:
-               lttng_msgpack_write_signed_integer(writer, output->u.s);
+               ret = lttng_msgpack_write_signed_integer(writer, output->u.s);
                break;
        case LTTNG_INTERPRETER_TYPE_U64:
-               lttng_msgpack_write_unsigned_integer(writer, output->u.u);
+               ret = lttng_msgpack_write_unsigned_integer(writer, output->u.u);
                break;
        case LTTNG_INTERPRETER_TYPE_DOUBLE:
-               lttng_msgpack_write_double(writer, output->u.d);
+               ret = lttng_msgpack_write_double(writer, output->u.d);
                break;
        case LTTNG_INTERPRETER_TYPE_STRING:
-               lttng_msgpack_write_str(writer, output->u.str.str);
+               ret = lttng_msgpack_write_str(writer, output->u.str.str);
                break;
        case LTTNG_INTERPRETER_TYPE_SEQUENCE:
-               capture_sequence(writer, output);
+               ret = capture_sequence(writer, output);
                break;
        case LTTNG_INTERPRETER_TYPE_SIGNED_ENUM:
        case LTTNG_INTERPRETER_TYPE_UNSIGNED_ENUM:
-               capture_enum(writer, output);
+               ret = capture_enum(writer, output);
                break;
        default:
-               abort();
+               CRIT("Unknown capture output type\n");
+               ret = -1;
        }
+       return ret;
 }
 
 static
-void notification_append_empty_capture(
+int notification_append_empty_capture(
                struct lttng_event_notifier_notification *notif)
 {
-       lttng_msgpack_write_nil(&notif->writer);
+       return lttng_msgpack_write_nil(&notif->writer);
 }
 
 static void record_error(const struct lttng_ust_event_notifier *event_notifier)
@@ -371,7 +413,10 @@ void lttng_event_notifier_notification_send(
         */
        struct lttng_event_notifier_notification notif = {0};
 
-       notification_init(&notif, event_notifier);
+       if (notification_init(&notif, event_notifier)) {
+               record_error(event_notifier);
+               goto end;
+       }
 
        if (caa_unlikely(notif_ctx->eval_capture)) {
                struct lttng_ust_bytecode_runtime *capture_bc_runtime;
@@ -385,12 +430,23 @@ void lttng_event_notifier_notification_send(
                cds_list_for_each_entry_rcu(capture_bc_runtime,
                                &event_notifier->priv->capture_bytecode_runtime_head, node) {
                        struct lttng_interpreter_output output;
+                       uint8_t *save_pos;
+                       int ret;
 
+                       lttng_msgpack_save_writer_pos(&notif.writer, &save_pos);
                        if (capture_bc_runtime->interpreter_func(capture_bc_runtime,
                                        stack_data, probe_ctx, &output) == LTTNG_UST_BYTECODE_INTERPRETER_OK)
-                               notification_append_capture(&notif, &output);
+                               ret = notification_append_capture(&notif, &output);
                        else
-                               notification_append_empty_capture(&notif);
+                               ret = notification_append_empty_capture(&notif);
+                       if (ret) {
+                               /*
+                                * On append capture error, skip the field
+                                * capture by restoring the msgpack writer
+                                * position.
+                                */
+                               lttng_msgpack_restore_writer_pos(&notif.writer, save_pos);
+                       }
                }
        }
 
@@ -399,4 +455,6 @@ void lttng_event_notifier_notification_send(
         * sessiond.
         */
        notification_send(&notif, event_notifier);
+end:
+       return;
 }