From c02eb85907f43fe17d97604633ae9aa1ed2afb36 Mon Sep 17 00:00:00 2001 From: Mathieu Desnoyers Date: Mon, 29 Jun 2020 19:52:08 -0400 Subject: [PATCH] Fix: callstack context memory corruption commit ceabb767180e "tracepoint: Refactor representation of nested types" introduces two context fields for callstack contexts. Keeping a pointer to the first field is not valid when adding the second context field to the array, because the array is reallocated. Fix this by introducing new context APIs which operate on indexes rather than pointers: - lttng_append_context_index, - lttng_get_context_field_from_index, - lttng_remove_context_field_index. Add a NULL check to lttng_find_context so it can be used before adding the first context. Signed-off-by: Mathieu Desnoyers --- include/lttng/events.h | 4 +++ src/lttng-context-callstack.c | 33 ++++++++++++--------- src/lttng-context.c | 55 ++++++++++++++++++++++++++++++----- 3 files changed, 71 insertions(+), 21 deletions(-) diff --git a/include/lttng/events.h b/include/lttng/events.h index f010ff76..80358e92 100644 --- a/include/lttng/events.h +++ b/include/lttng/events.h @@ -683,11 +683,15 @@ extern struct lttng_ctx *lttng_static_ctx; int lttng_context_init(void); void lttng_context_exit(void); struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx); +ssize_t lttng_append_context_index(struct lttng_ctx **ctx_p); +struct lttng_ctx_field *lttng_get_context_field_from_index(struct lttng_ctx *ctx, + size_t index); void lttng_context_update(struct lttng_ctx *ctx); int lttng_find_context(struct lttng_ctx *ctx, const char *name); int lttng_get_context_index(struct lttng_ctx *ctx, const char *name); void lttng_remove_context_field(struct lttng_ctx **ctx, struct lttng_ctx_field *field); +void lttng_remove_context_field_index(struct lttng_ctx **ctx_p, size_t index); void lttng_destroy_context(struct lttng_ctx *ctx); int lttng_add_pid_to_ctx(struct lttng_ctx **ctx); int lttng_add_cpu_id_to_ctx(struct lttng_ctx **ctx); diff --git a/src/lttng-context-callstack.c b/src/lttng-context-callstack.c index 7b9e6512..6b75772e 100644 --- a/src/lttng-context-callstack.c +++ b/src/lttng-context-callstack.c @@ -105,6 +105,7 @@ int __lttng_add_callstack_generic(struct lttng_ctx **ctx, const char *ctx_name = lttng_cs_ctx_mode_name(mode); const char *ctx_length_name = lttng_cs_ctx_mode_length_name(mode); struct lttng_ctx_field *length_field, *sequence_field; + ssize_t length_index, sequence_index; struct lttng_event_field *field; struct field_data *fdata; int ret; @@ -112,18 +113,22 @@ int __lttng_add_callstack_generic(struct lttng_ctx **ctx, ret = init_type(mode); if (ret) return ret; - length_field = lttng_append_context(ctx); - if (!length_field) - return -ENOMEM; - sequence_field = lttng_append_context(ctx); - if (!sequence_field) { - lttng_remove_context_field(ctx, length_field); - return -ENOMEM; + if (lttng_find_context(*ctx, ctx_name)) + return -EEXIST; + length_index = lttng_append_context_index(ctx); + if (length_index < 0) { + ret = -ENOMEM; + goto error_length; } - if (lttng_find_context(*ctx, ctx_name)) { - ret = -EEXIST; - goto error_find; + sequence_index = lttng_append_context_index(ctx); + if (sequence_index < 0) { + ret = -ENOMEM; + goto error_sequence; } + length_field = lttng_get_context_field_from_index(*ctx, length_index); + WARN_ON_ONCE(!length_field); + sequence_field = lttng_get_context_field_from_index(*ctx, sequence_index); + WARN_ON_ONCE(!sequence_field); fdata = field_data_create(mode); if (!fdata) { ret = -ENOMEM; @@ -156,10 +161,10 @@ int __lttng_add_callstack_generic(struct lttng_ctx **ctx, return 0; error_create: - field_data_free(fdata); -error_find: - lttng_remove_context_field(ctx, sequence_field); - lttng_remove_context_field(ctx, length_field); + lttng_remove_context_field_index(ctx, sequence_index); +error_sequence: + lttng_remove_context_field_index(ctx, length_index); +error_length: return ret; } diff --git a/src/lttng-context.c b/src/lttng-context.c index eb5e5d11..9d6b71e0 100644 --- a/src/lttng-context.c +++ b/src/lttng-context.c @@ -29,6 +29,8 @@ int lttng_find_context(struct lttng_ctx *ctx, const char *name) { unsigned int i; + if (!ctx) + return 0; for (i = 0; i < ctx->nr_fields; i++) { /* Skip allocated (but non-initialized) contexts */ if (!ctx->fields[i].event_field.name) @@ -63,18 +65,27 @@ int lttng_get_context_index(struct lttng_ctx *ctx, const char *name) } EXPORT_SYMBOL_GPL(lttng_get_context_index); +struct lttng_ctx_field *lttng_get_context_field_from_index(struct lttng_ctx *ctx, + size_t index) +{ + if (index >= ctx->nr_fields) + return NULL; + return &ctx->fields[index]; +} +EXPORT_SYMBOL_GPL(lttng_get_context_field_from_index); + /* * Note: as we append context information, the pointer location may change. */ -struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p) +ssize_t lttng_append_context_index(struct lttng_ctx **ctx_p) { - struct lttng_ctx_field *field; struct lttng_ctx *ctx; + ssize_t pos = -1; if (!*ctx_p) { *ctx_p = kzalloc(sizeof(struct lttng_ctx), GFP_KERNEL); if (!*ctx_p) - return NULL; + goto end; (*ctx_p)->largest_align = 1; } ctx = *ctx_p; @@ -84,15 +95,29 @@ struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p) ctx->allocated_fields = max_t(size_t, 1, 2 * ctx->allocated_fields); new_fields = lttng_kvzalloc(ctx->allocated_fields * sizeof(struct lttng_ctx_field), GFP_KERNEL); if (!new_fields) - return NULL; + goto end; if (ctx->fields) memcpy(new_fields, ctx->fields, sizeof(*ctx->fields) * ctx->nr_fields); lttng_kvfree(ctx->fields); ctx->fields = new_fields; } - field = &ctx->fields[ctx->nr_fields]; - ctx->nr_fields++; - return field; + pos = ctx->nr_fields++; +end: + return pos; +} +EXPORT_SYMBOL_GPL(lttng_append_context_index); + +/* + * Note: as we append context information, the pointer location may change. + */ +struct lttng_ctx_field *lttng_append_context(struct lttng_ctx **ctx_p) +{ + ssize_t pos; + + pos = lttng_append_context_index(ctx_p); + if (pos < 0) + return NULL; + return &(*ctx_p)->fields[pos]; } EXPORT_SYMBOL_GPL(lttng_append_context); @@ -180,6 +205,22 @@ void lttng_context_update(struct lttng_ctx *ctx) ctx->largest_align = largest_align >> 3; /* bits to bytes */ } +/* Keep same order. */ +void lttng_remove_context_field_index(struct lttng_ctx **ctx_p, size_t index) +{ + struct lttng_ctx *ctx = *ctx_p; + + WARN_ON_ONCE(ctx->nr_fields >= index); + if (index != ctx->nr_fields - 1) { + memmove(&ctx->fields[index], &ctx->fields[index + 1], + (ctx->nr_fields - index - 1) * sizeof(struct lttng_ctx_field)); + } + /* Clear last item. */ + memset(&ctx->fields[ctx->nr_fields - 1], 0, sizeof(struct lttng_ctx_field)); + ctx->nr_fields--; +} +EXPORT_SYMBOL_GPL(lttng_remove_context_field_index); + /* * Remove last context field. */ -- 2.34.1