From c2afda67713866265294d11c72deca6edba05969 Mon Sep 17 00:00:00 2001 From: Jonathan Rajotte Date: Wed, 12 Jan 2022 18:18:08 -0500 Subject: [PATCH] Fix: liblttng-ctl comm: lttng_event_field is not packed MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Jérémie Galarneau Change-Id: I280d9809d110237574e2606ee93a7aeba41e704e --- include/lttng/event-internal.h | 29 +++ src/bin/lttng-sessiond/client.c | 30 +-- src/bin/lttng-sessiond/cmd.c | 32 +++- src/bin/lttng-sessiond/cmd.h | 5 +- src/common/event.c | 328 ++++++++++++++++++++++++++++++++ src/lib/lttng-ctl/lttng-ctl.c | 46 ++++- 6 files changed, 440 insertions(+), 30 deletions(-) diff --git a/include/lttng/event-internal.h b/include/lttng/event-internal.h index 54f33fdf1..898609879 100644 --- a/include/lttng/event-internal.h +++ b/include/lttng/event-internal.h @@ -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 @@ -170,10 +184,25 @@ int lttng_event_context_serialize(struct lttng_event_context *context, LTTNG_HIDDEN void lttng_event_context_destroy(struct lttng_event_context *context); +LTTNG_HIDDEN +ssize_t lttng_event_field_create_from_buffer( + const struct lttng_buffer_view *view, + struct lttng_event_field **field); + +LTTNG_HIDDEN +int lttng_event_field_serialize(const struct lttng_event_field *field, + struct lttng_dynamic_buffer *buffer); + LTTNG_HIDDEN enum lttng_error_code lttng_events_create_and_flatten_from_buffer( const struct lttng_buffer_view *view, unsigned int count, struct lttng_event **events); +LTTNG_HIDDEN +enum lttng_error_code lttng_event_fields_create_and_flatten_from_buffer( + const struct lttng_buffer_view *view, + unsigned int count, + struct lttng_event_field **fields); + #endif /* LTTNG_EVENT_INTERNAL_H */ diff --git a/src/bin/lttng-sessiond/client.c b/src/bin/lttng-sessiond/client.c index d6de4bfbd..356381142 100644 --- a/src/bin/lttng-sessiond/client.c +++ b/src/bin/lttng-sessiond/client.c @@ -1223,26 +1223,28 @@ skip_domain: } case LTTNG_LIST_TRACEPOINT_FIELDS: { - struct lttng_event_field *fields; - ssize_t nb_fields; + enum lttng_error_code ret_code; + unsigned int nb_fields; + struct lttcomm_list_command_header cmd_header; 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, &payload, &nb_fields); 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 (nb_fields > UINT32_MAX) { + ret = LTTNG_ERR_OVERFLOW; + goto error; + } + + cmd_header.count = nb_fields; + + ret = setup_lttng_msg(cmd_ctx, payload.data, payload.size, + &cmd_header, sizeof(cmd_header)); if (ret < 0) { goto setup_error; diff --git a/src/bin/lttng-sessiond/cmd.c b/src/bin/lttng-sessiond/cmd.c index 5a6414ebb..78f1f053d 100644 --- a/src/bin/lttng-sessiond/cmd.c +++ b/src/bin/lttng-sessiond/cmd.c @@ -2650,31 +2650,43 @@ 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_dynamic_buffer *buffer, unsigned int *nb_fields) { + enum lttng_error_code ret_code; int ret; - ssize_t nb_fields = 0; + unsigned int i; + struct lttng_event_field *fields = NULL; 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], buffer); + if (ret) { + ret_code = -LTTNG_ERR_NOMEM; + goto error; + } + } + + 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( diff --git a/src/bin/lttng-sessiond/cmd.h b/src/bin/lttng-sessiond/cmd.h index 6b693c06a..d7106349a 100644 --- a/src/bin/lttng-sessiond/cmd.h +++ b/src/bin/lttng-sessiond/cmd.h @@ -112,8 +112,9 @@ 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_dynamic_buffer *buffer, + unsigned int *nb_fields); enum lttng_error_code cmd_list_tracepoints(enum lttng_domain_type domain, struct lttng_dynamic_buffer *buffer, unsigned int *nb_tracepoints); diff --git a/src/common/event.c b/src/common/event.c index 0f6ac9a8c..b90bc2a2f 100644 --- a/src/common/event.c +++ b/src/common/event.c @@ -1325,6 +1325,182 @@ 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; +} + +LTTNG_HIDDEN +ssize_t lttng_event_field_create_from_buffer( + const struct lttng_buffer_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, offset, sizeof(*comm)); + + if (!lttng_buffer_view_is_valid(&comm_view)) { + ret = -1; + goto end; + } + /* lttng_event_field_comm header */ + comm = (const struct lttng_event_field_comm *) view->data; + offset += sizeof(*comm); + } + + local_event_field = zmalloc(sizeof(*local_event_field)); + if (!local_event_field) { + ret = -1; + goto end; + } + + local_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, 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 */ + { + const struct lttng_buffer_view event_view = + lttng_buffer_view_from_view( + view, offset, comm->event_len); + + if (!lttng_buffer_view_is_valid(&event_view)) { + ret = -1; + goto end; + } + ret = lttng_event_create_from_buffer(&event_view, &event, NULL, + NULL, NULL, -1); + if (ret != comm->event_len) { + abort(); + } + 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; +} + +LTTNG_HIDDEN +int lttng_event_field_serialize(const struct lttng_event_field *field, + struct lttng_dynamic_buffer *buffer) +{ + 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(buffer); + + /* Save the header location for later in-place header update. */ + header_offset = 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( + buffer, &event_field_comm, sizeof(event_field_comm)); + if (ret) { + goto end; + } + + /* Field name */ + ret = lttng_dynamic_buffer_append(buffer, field->field_name, name_len); + if (ret) { + goto end; + } + + size_before_event = buffer->size; + ret = lttng_event_serialize( + &field->event, 0, NULL, NULL, 0, 0, buffer, NULL); + if (ret) { + ret = -1; + goto end; + } + + /* Update the event len. */ + header = (struct lttng_event_field_comm *) ((char *) buffer->data + + header_offset); + header->event_len = 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) { @@ -1640,3 +1816,155 @@ 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 = + 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_buffer( + const struct lttng_buffer_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 = 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; + const struct lttng_buffer_view event_field_view = + lttng_buffer_view_from_view(view, offset, -1); + + event_field_size = lttng_event_field_create_from_buffer( + &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->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; +} + +LTTNG_HIDDEN +enum lttng_error_code lttng_event_fields_create_and_flatten_from_buffer( + const struct lttng_buffer_view *view, + unsigned int count, + struct lttng_event_field **fields) +{ + enum lttng_error_code ret; + struct lttng_dynamic_pointer_array *local_event_fields = NULL; + + ret = event_field_list_create_from_buffer( + view, count, &local_event_fields); + if (ret != LTTNG_OK) { + goto end; + } + + ret = flatten_lttng_event_fields(local_event_fields, fields); + if (ret != LTTNG_OK) { + goto end; + } +end: + if (local_event_fields) { + lttng_dynamic_pointer_array_reset(local_event_fields); + free(local_event_fields); + } + return ret; +} diff --git a/src/lib/lttng-ctl/lttng-ctl.c b/src/lib/lttng-ctl/lttng-ctl.c index a387202a6..fd5d9e3d6 100644 --- a/src/lib/lttng-ctl/lttng-ctl.c +++ b/src/lib/lttng-ctl/lttng-ctl.c @@ -1652,8 +1652,13 @@ end: int lttng_list_tracepoint_fields(struct lttng_handle *handle, struct lttng_event_field **fields) { - int ret; + enum lttng_error_code ret_code; + int ret, total_payload_received; struct lttcomm_session_msg lsm; + char *reception_buffer = NULL; + struct lttcomm_list_command_header *cmd_header = NULL; + size_t cmd_header_len; + unsigned int nb_event_fields = 0; if (handle == NULL) { return -LTTNG_ERR_INVALID; @@ -1663,12 +1668,45 @@ int lttng_list_tracepoint_fields(struct lttng_handle *handle, lsm.cmd_type = LTTNG_LIST_TRACEPOINT_FIELDS; COPY_DOMAIN_PACKED(lsm.domain, handle->domain); - ret = lttng_ctl_ask_sessiond(&lsm, (void **) fields); + ret = lttng_ctl_ask_sessiond_fds_varlen(&lsm, NULL, 0, NULL, 0, + (void **) &reception_buffer, (void **) &cmd_header, + &cmd_header_len); if (ret < 0) { - return ret; + goto end; } - return ret / sizeof(struct lttng_event_field); + total_payload_received = ret; + + if (!cmd_header) { + ret = -LTTNG_ERR_UNK; + goto end; + } + + if (cmd_header->count > INT_MAX) { + ret = -LTTNG_ERR_OVERFLOW; + goto end; + } + + nb_event_fields = cmd_header->count; + + { + const struct lttng_buffer_view view = + lttng_buffer_view_init(reception_buffer, 0, total_payload_received); + + ret_code = lttng_event_fields_create_and_flatten_from_buffer( + &view, nb_event_fields, fields); + if (ret_code != LTTNG_OK) { + ret = -ret_code; + goto end; + } + } + + ret = nb_event_fields; + +end: + free(cmd_header); + free(reception_buffer); + return ret; } /* -- 2.34.1