Fix: liblttng-ctl comm: lttng_event_field is not packed
authorJonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Wed, 12 Jan 2022 23:18:08 +0000 (18:18 -0500)
committerJérémie Galarneau <jeremie.galarneau@efficios.com>
Mon, 28 Feb 2022 21:46:11 +0000 (16:46 -0500)
Observed issue
==============

For MI testing where the lttng-sessiond is 64 bit and the lttng CLI is
32 bit, the tracepoint field listing fails with partial garbage output.

The size of the struct differs between bitness for x86-64 and x86
leading to serialization/deserialization problem across client
(liblttng-ctl) and lttng-sessiond.

sizeof(struct lttng_event_field):

  x86: 1136
  x86-64: 1144

The struct cannot be marked as LTTNG_PACKED since it is part of the API.

Solution
========

Adopt a similar pattern to the new APIs with a "serialize" &
"create_from_buffer" approach. The only particularity is that we need to
flatten the event_field on listing.

Most of the complexity is moved to `src/common/event.c`

Known drawbacks
=========

None.

Signed-off-by: Jonathan Rajotte <jonathan.rajotte-julien@efficios.com>
Signed-off-by: Jérémie Galarneau <jeremie.galarneau@efficios.com>
Change-Id: I280d9809d110237574e2606ee93a7aeba41e704e

include/lttng/event-internal.h
src/bin/lttng-sessiond/client.cpp
src/bin/lttng-sessiond/cmd.cpp
src/bin/lttng-sessiond/cmd.h
src/common/event.cpp
src/lib/lttng-ctl/lttng-ctl.cpp

index 6eb7ab37a7c67db671f59199c769ce54e79759f6..52f7f7857981c3b684fd6946b845bf2f54068284 100644 (file)
@@ -121,6 +121,20 @@ struct lttng_event_context_app_comm {
        char payload[];
 } LTTNG_PACKED;
 
+struct lttng_event_field_comm {
+       uint8_t type;
+       uint8_t nowrite;
+       /* Includes terminator `\0`. */
+       uint32_t name_len;
+       uint32_t event_len;
+
+       /*
+        * - name [name_len]
+        * - lttng_event object
+        */
+       char payload[];
+} LTTNG_PACKED;
+
 struct lttng_event_extended {
        /*
         * exclusions and filter_expression are only set when the lttng_event
@@ -167,4 +181,16 @@ enum lttng_error_code lttng_events_create_and_flatten_from_payload(
                unsigned int count,
                struct lttng_event **events);
 
+ssize_t lttng_event_field_create_from_payload(
+               struct lttng_payload_view *view,
+               struct lttng_event_field **field);
+
+int lttng_event_field_serialize(const struct lttng_event_field *field,
+               struct lttng_payload *payload);
+
+enum lttng_error_code lttng_event_fields_create_and_flatten_from_payload(
+               struct lttng_payload_view *view,
+               unsigned int count,
+               struct lttng_event_field **fields);
+
 #endif /* LTTNG_EVENT_INTERNAL_H */
index abd26fcb96f755b9687cfa2cdb23342e387d2bff..16f1b1bcd2eae31b0c8e2c09adacef30acb73948 100644 (file)
@@ -1642,30 +1642,33 @@ skip_domain:
        }
        case LTTNG_LIST_TRACEPOINT_FIELDS:
        {
-               struct lttng_event_field *fields;
-               ssize_t nb_fields;
+               enum lttng_error_code ret_code;
+               size_t original_payload_size;
+               size_t payload_size;
+               const size_t command_header_size = sizeof(struct lttcomm_list_command_header);
+
+               ret = setup_empty_lttng_msg(cmd_ctx);
+               if (ret) {
+                       ret = LTTNG_ERR_NOMEM;
+                       goto setup_error;
+               }
+
+               original_payload_size = cmd_ctx->reply_payload.buffer.size;
+               ERR("original payload size = %i", (int) original_payload_size);
 
                session_lock_list();
-               nb_fields = cmd_list_tracepoint_fields(cmd_ctx->lsm.domain.type,
-                               &fields);
+               ret_code = cmd_list_tracepoint_fields(
+                               cmd_ctx->lsm.domain.type, &cmd_ctx->reply_payload);
                session_unlock_list();
-               if (nb_fields < 0) {
-                       /* Return value is a negative lttng_error_code. */
-                       ret = -nb_fields;
+               if (ret_code != LTTNG_OK) {
+                       ret = (int) ret_code;
                        goto error;
                }
 
-               /*
-                * Setup lttng message with payload size set to the event list size in
-                * bytes and then copy list into the llm payload.
-                */
-               ret = setup_lttng_msg_no_cmd_header(cmd_ctx, fields,
-                               sizeof(struct lttng_event_field) * nb_fields);
-               free(fields);
-
-               if (ret < 0) {
-                       goto setup_error;
-               }
+               payload_size = cmd_ctx->reply_payload.buffer.size -
+                               command_header_size - original_payload_size;
+               ERR("payload size = %i", (int) payload_size);
+               update_lttng_msg(cmd_ctx, command_header_size, payload_size);
 
                ret = LTTNG_OK;
                break;
index f4ce9a8829f70b77ece1e958b1ac42767aa41c8d..0627ba5c8bd5f83ca62d9403cddea283a2ce6f8f 100644 (file)
@@ -2559,31 +2559,70 @@ error:
 /*
  * Command LTTNG_LIST_TRACEPOINT_FIELDS processed by the client thread.
  */
-ssize_t cmd_list_tracepoint_fields(enum lttng_domain_type domain,
-               struct lttng_event_field **fields)
+enum lttng_error_code cmd_list_tracepoint_fields(enum lttng_domain_type domain,
+               struct lttng_payload *reply)
 {
+       enum lttng_error_code ret_code;
        int ret;
-       ssize_t nb_fields = 0;
+       unsigned int i, nb_fields;
+       struct lttng_event_field *fields = NULL;
+       struct lttcomm_list_command_header reply_command_header = {};
+       size_t reply_command_header_offset;
+
+       assert(reply);
+
+       /* Reserve space for command reply header. */
+       reply_command_header_offset = reply->buffer.size;
+       ret = lttng_dynamic_buffer_set_size(&reply->buffer,
+                       reply_command_header_offset +
+                               sizeof(struct lttcomm_list_command_header));
+       if (ret) {
+               ret_code = LTTNG_ERR_NOMEM;
+               goto error;
+       }
 
        switch (domain) {
        case LTTNG_DOMAIN_UST:
-               nb_fields = ust_app_list_event_fields(fields);
-               if (nb_fields < 0) {
-                       ret = LTTNG_ERR_UST_LIST_FAIL;
+               ret = ust_app_list_event_fields(&fields);
+               if (ret < 0) {
+                       ret_code = LTTNG_ERR_UST_LIST_FAIL;
                        goto error;
                }
+
                break;
        case LTTNG_DOMAIN_KERNEL:
        default:        /* fall-through */
-               ret = LTTNG_ERR_UND;
+               ret_code = LTTNG_ERR_UND;
                goto error;
        }
 
-       return nb_fields;
+       nb_fields = ret;
+
+       for (i = 0; i < nb_fields; i++) {
+               ret = lttng_event_field_serialize(&fields[i], reply);
+               if (ret) {
+                       ret_code = LTTNG_ERR_NOMEM;
+                       goto error;
+               }
+       }
+
+       if (nb_fields > UINT32_MAX) {
+               ERR("Tracepoint field count would overflow the tracepoint field listing command's reply");
+               ret_code = LTTNG_ERR_OVERFLOW;
+               goto error;
+       }
+
+       /* Update command reply header. */
+       reply_command_header.count = (uint32_t) nb_fields;
+
+       memcpy(reply->buffer.data + reply_command_header_offset, &reply_command_header,
+                       sizeof(reply_command_header));
+
+       ret_code = LTTNG_OK;
 
 error:
-       /* Return negative value to differentiate return code */
-       return -ret;
+       free(fields);
+       return ret_code;
 }
 
 enum lttng_error_code cmd_list_syscalls(
index 69389e5fa258ce9ee2b88d6a43980da0d0e52ceb..bd2d939741d2a40adf080c528b9d267ec57d78a5 100644 (file)
@@ -121,8 +121,8 @@ ssize_t cmd_list_domains(struct ltt_session *session,
                struct lttng_domain **domains);
 void cmd_list_lttng_sessions(struct lttng_session *sessions,
                size_t session_count, uid_t uid, gid_t gid);
-ssize_t cmd_list_tracepoint_fields(enum lttng_domain_type domain,
-               struct lttng_event_field **fields);
+enum lttng_error_code cmd_list_tracepoint_fields(enum lttng_domain_type domain,
+               struct lttng_payload *reply);
 enum lttng_error_code cmd_list_tracepoints(enum lttng_domain_type domain,
                struct lttng_payload *reply_payload);
 ssize_t cmd_snapshot_list_outputs(struct ltt_session *session,
index 502ccdcf27423c0467750d0bb26b72ea70df9b64..c3a2c0eb7203566924b0240559f3fe4bbc46a272 100644 (file)
@@ -1278,6 +1278,192 @@ void lttng_event_context_destroy(struct lttng_event_context *context)
        free(context);
 }
 
+/*
+ * This is a specialized populate for lttng_event_field since it ignores
+ * the extension field of the lttng_event struct and simply copies what it can
+ * to the internal struct lttng_event of a lttng_event_field.
+ */
+static void lttng_event_field_populate_lttng_event_from_event(
+               const struct lttng_event *src, struct lttng_event *destination)
+{
+       memcpy(destination, src, sizeof(*destination));
+
+       /* Remove all possible dynamic data from the destination event rule. */
+       destination->extended.ptr = NULL;
+}
+
+ssize_t lttng_event_field_create_from_payload(
+               struct lttng_payload_view *view,
+               struct lttng_event_field **field)
+{
+       ssize_t ret, offset = 0;
+       struct lttng_event_field *local_event_field = NULL;
+       struct lttng_event *event = NULL;
+       const struct lttng_event_field_comm *comm;
+       const char* name = NULL;
+
+       assert(field);
+       assert(view);
+
+       {
+               const struct lttng_buffer_view comm_view =
+                               lttng_buffer_view_from_view(
+                                               &view->buffer, offset,
+                                               sizeof(*comm));
+
+               if (!lttng_buffer_view_is_valid(&comm_view)) {
+                       ret = -1;
+                       goto end;
+               }
+
+               /* lttng_event_field_comm header */
+               comm = (const lttng_event_field_comm *) comm_view.data;
+               offset += sizeof(*comm);
+       }
+
+       local_event_field = (struct lttng_event_field *)
+                       zmalloc(sizeof(*local_event_field));
+       if (!local_event_field) {
+               ret = -1;
+               goto end;
+       }
+
+       local_event_field->type = (lttng_event_field_type) comm->type;
+       local_event_field->nowrite = comm->nowrite;
+
+       /* Field name */
+       {
+               const struct lttng_buffer_view name_view =
+                               lttng_buffer_view_from_view(
+                                               &view->buffer, offset,
+                                               comm->name_len);
+
+               if (!lttng_buffer_view_is_valid(&name_view)) {
+                       ret = -1;
+                       goto end;
+               }
+
+               name = name_view.data;
+
+               if (!lttng_buffer_view_contains_string(&name_view,
+                               name_view.data, comm->name_len)) {
+                       ret = -1;
+                       goto end;
+               }
+
+               if (comm->name_len > LTTNG_SYMBOL_NAME_LEN - 1) {
+                       /* Name is too long.*/
+                       ret = -1;
+                       goto end;
+               }
+
+               offset += comm->name_len;
+       }
+
+       /* Event */
+       {
+               struct lttng_payload_view event_view =
+                               lttng_payload_view_from_view(
+                                               view, offset,
+                                               comm->event_len);
+
+               if (!lttng_payload_view_is_valid(&event_view)) {
+                       ret = -1;
+                       goto end;
+               }
+
+               ret = lttng_event_create_from_payload(&event_view, &event, NULL,
+                               NULL, NULL);
+               if (ret != comm->event_len) {
+                       ret = -1;
+                       goto end;
+               }
+
+               offset += ret;
+       }
+
+       assert(name);
+       assert(event);
+
+       if (lttng_strncpy(local_event_field->field_name, name , LTTNG_SYMBOL_NAME_LEN)) {
+               ret = -1;
+               goto end;
+       }
+
+       lttng_event_field_populate_lttng_event_from_event(
+                       event, &local_event_field->event);
+
+       *field = local_event_field;
+       local_event_field = NULL;
+       ret = offset;
+end:
+       lttng_event_destroy(event);
+       free(local_event_field);
+       return ret;
+}
+
+int lttng_event_field_serialize(const struct lttng_event_field *field,
+               struct lttng_payload *payload)
+{
+       int ret;
+       size_t header_offset, size_before_event;
+       size_t name_len;
+       struct lttng_event_field_comm event_field_comm = { 0 };
+       struct lttng_event_field_comm *header;
+
+       assert(field);
+       assert(payload);
+
+       /* Save the header location for later in-place header update. */
+       header_offset = payload->buffer.size;
+
+       name_len = strnlen(field->field_name, LTTNG_SYMBOL_NAME_LEN);
+       if (name_len == LTTNG_SYMBOL_NAME_LEN) {
+               /* Event name is not NULL-terminated. */
+               ret = -1;
+               goto end;
+       }
+
+       /* Add null termination. */
+       name_len += 1;
+
+       event_field_comm.type = field->type;
+       event_field_comm.nowrite = (uint8_t)field->nowrite;
+       event_field_comm.name_len = name_len;
+
+       /* Header */
+       ret = lttng_dynamic_buffer_append(
+                       &payload->buffer, &event_field_comm,
+                       sizeof(event_field_comm));
+       if (ret) {
+               goto end;
+       }
+
+       /* Field name */
+       ret = lttng_dynamic_buffer_append(&payload->buffer, field->field_name,
+                       name_len);
+       if (ret) {
+               goto end;
+       }
+
+       size_before_event = payload->buffer.size;
+       ret = lttng_event_serialize(
+                       &field->event, 0, NULL, NULL, 0, 0, payload);
+       if (ret) {
+               ret = -1;
+               goto end;
+       }
+
+       /* Update the event len. */
+       header = (struct lttng_event_field_comm *)
+                       ((char *) payload->buffer.data +
+                               header_offset);
+       header->event_len = payload->buffer.size - size_before_event;
+
+end:
+       return ret;
+}
+
 static enum lttng_error_code compute_flattened_size(
                struct lttng_dynamic_pointer_array *events, size_t *size)
 {
@@ -1595,3 +1781,158 @@ end:
        lttng_dynamic_pointer_array_reset(&local_events);
        return ret;
 }
+
+static enum lttng_error_code flatten_lttng_event_fields(
+               struct lttng_dynamic_pointer_array *event_fields,
+               struct lttng_event_field **flattened_event_fields)
+{
+       int ret, i;
+       enum lttng_error_code ret_code;
+       size_t storage_req = 0;
+       struct lttng_dynamic_buffer local_flattened_event_fields;
+       int nb_event_field;
+
+       assert(event_fields);
+       assert(flattened_event_fields);
+
+       lttng_dynamic_buffer_init(&local_flattened_event_fields);
+       nb_event_field = lttng_dynamic_pointer_array_get_count(event_fields);
+
+       /*
+        * Here even if the event field contains a `struct lttng_event` that
+        * could contain dynamic data, in reality it is not the case.
+        * Dynamic data is not present. Here the flattening is mostly a direct
+        * memcpy. This is less than ideal but this code is still better than
+        * direct usage of an unpacked lttng_event_field array.
+        */
+       storage_req += sizeof(struct lttng_event_field) * nb_event_field;
+
+       lttng_dynamic_buffer_init(&local_flattened_event_fields);
+
+       /*
+        * We must ensure that "local_flattened_event_fields" is never resized
+        * so as to preserve the validity of the flattened objects.
+        */
+       ret = lttng_dynamic_buffer_set_capacity(
+                       &local_flattened_event_fields, storage_req);
+       if (ret) {
+               ret_code = LTTNG_ERR_NOMEM;
+               goto end;
+       }
+
+       for (i = 0; i < nb_event_field; i++) {
+               const struct lttng_event_field *element =
+                               (const struct lttng_event_field *)
+                                       lttng_dynamic_pointer_array_get_pointer(
+                                               event_fields, i);
+
+               if (!element) {
+                       ret_code = LTTNG_ERR_FATAL;
+                       goto end;
+               }
+               ret = lttng_dynamic_buffer_append(&local_flattened_event_fields,
+                               element, sizeof(struct lttng_event_field));
+               if (ret) {
+                       ret_code = LTTNG_ERR_NOMEM;
+                       goto end;
+               }
+       }
+
+       /* Don't reset local_flattened_channels buffer as we return its content. */
+       *flattened_event_fields = (struct lttng_event_field *) local_flattened_event_fields.data;
+       lttng_dynamic_buffer_init(&local_flattened_event_fields);
+       ret_code = LTTNG_OK;
+end:
+       lttng_dynamic_buffer_reset(&local_flattened_event_fields);
+       return ret_code;
+}
+
+static enum lttng_error_code event_field_list_create_from_payload(
+               struct lttng_payload_view *view,
+               unsigned int count,
+               struct lttng_dynamic_pointer_array **event_field_list)
+{
+       enum lttng_error_code ret_code;
+       int ret, offset = 0;
+       unsigned int i;
+       struct lttng_dynamic_pointer_array *list = NULL;
+
+       assert(view);
+       assert(event_field_list);
+
+       list = (struct lttng_dynamic_pointer_array *) zmalloc(sizeof(*list));
+       if (!list) {
+               ret_code = LTTNG_ERR_NOMEM;
+               goto end;
+       }
+
+       lttng_dynamic_pointer_array_init(list, free);
+
+       for (i = 0; i < count; i++) {
+               ssize_t event_field_size;
+               struct lttng_event_field *field = NULL;
+               struct lttng_payload_view event_field_view =
+                               lttng_payload_view_from_view(view, offset, -1);
+
+               event_field_size = lttng_event_field_create_from_payload(
+                               &event_field_view, &field);
+               if (event_field_size < 0) {
+                       ret_code = LTTNG_ERR_INVALID;
+                       goto end;
+               }
+
+               /* Lifetime and management of the object is now bound to the array. */
+               ret = lttng_dynamic_pointer_array_add_pointer(list, field);
+               if (ret) {
+                       free(field);
+                       ret_code = LTTNG_ERR_NOMEM;
+                       goto end;
+               }
+
+               offset += event_field_size;
+       }
+
+       if (view->buffer.size != offset) {
+               ret_code = LTTNG_ERR_INVALID;
+               goto end;
+       }
+
+       *event_field_list = list;
+       list = NULL;
+       ret_code = LTTNG_OK;
+
+end:
+       if (list) {
+               lttng_dynamic_pointer_array_reset(list);
+               free(list);
+       }
+
+       return ret_code;
+}
+
+enum lttng_error_code lttng_event_fields_create_and_flatten_from_payload(
+               struct lttng_payload_view *view,
+               unsigned int count,
+               struct lttng_event_field **fields)
+{
+       enum lttng_error_code ret_code;
+       struct lttng_dynamic_pointer_array *local_event_fields = NULL;
+
+       ret_code = event_field_list_create_from_payload(
+                       view, count, &local_event_fields);
+       if (ret_code != LTTNG_OK) {
+               goto end;
+       }
+
+       ret_code = flatten_lttng_event_fields(local_event_fields, fields);
+       if (ret_code != LTTNG_OK) {
+               goto end;
+       }
+end:
+       if (local_event_fields) {
+               lttng_dynamic_pointer_array_reset(local_event_fields);
+               free(local_event_fields);
+       }
+
+       return ret_code;
+}
index e671bae402c3d21927fc41ad17445a0778e192dc..c7fb313f90fb1eedfa0541b1014d8209473fd23e 100644 (file)
@@ -1715,23 +1715,74 @@ end:
 int lttng_list_tracepoint_fields(struct lttng_handle *handle,
                struct lttng_event_field **fields)
 {
+       enum lttng_error_code ret_code;
        int ret;
        struct lttcomm_session_msg lsm;
+       const struct lttcomm_list_command_header *cmd_header = NULL;
+       unsigned int nb_event_fields = 0;
+       struct lttng_payload reply;
 
        if (handle == NULL) {
-               return -LTTNG_ERR_INVALID;
+               ret = -LTTNG_ERR_INVALID;
+               goto end;
        }
 
+       lttng_payload_init(&reply);
+
        memset(&lsm, 0, sizeof(lsm));
        lsm.cmd_type = LTTNG_LIST_TRACEPOINT_FIELDS;
        COPY_DOMAIN_PACKED(lsm.domain, handle->domain);
 
-       ret = lttng_ctl_ask_sessiond(&lsm, (void **) fields);
-       if (ret < 0) {
-               return ret;
+       {
+               lttng_payload_view message_view =
+                               lttng_payload_view_init_from_buffer(
+                                       (const char *) &lsm, 0,
+                                       sizeof(lsm));
+
+               ret = lttng_ctl_ask_sessiond_payload(&message_view, &reply);
+               if (ret < 0) {
+                       goto end;
+               }
+       }
+
+       {
+               const lttng_buffer_view cmd_header_view =
+                               lttng_buffer_view_from_dynamic_buffer(
+                                       &reply.buffer, 0, sizeof(*cmd_header));
+
+               if (!lttng_buffer_view_is_valid(&cmd_header_view)) {
+                       ret = -LTTNG_ERR_INVALID_PROTOCOL;
+                       goto end;
+               }
+
+               cmd_header = (struct lttcomm_list_command_header *)
+                               cmd_header_view.data;
+       }
+
+       if (cmd_header->count > INT_MAX) {
+               ret = -LTTNG_ERR_OVERFLOW;
+               goto end;
+       }
+
+       nb_event_fields = cmd_header->count;
+
+       {
+               lttng_payload_view reply_view =
+                               lttng_payload_view_from_payload(&reply,
+                               sizeof(*cmd_header), -1);
+
+               ret_code = lttng_event_fields_create_and_flatten_from_payload(
+                               &reply_view, nb_event_fields, fields);
+               if (ret_code != LTTNG_OK) {
+                       ret = -ret_code;
+                       goto end;
+               }
        }
 
-       return ret / sizeof(struct lttng_event_field);
+       ret = nb_event_fields;
+
+end:
+       return ret;
 }
 
 /*
This page took 0.035131 seconds and 4 git commands to generate.