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 <mathieu.desnoyers@efficios.com>

diff --git a/include/lttng/events.h b/include/lttng/events.h
index f010ff7698ba707114497991146b56fe7c6cc9f6..80358e920d3d7a83f646c1da63d01c5364177d59 100644 (file)
--- 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 7b9e651272e4c3e5271928b8f051f96552401ba8..6b75772e4bec21e6333d4ab72bda264715619120 100644 (file)
--- 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 eb5e5d113cd6bad3bc65a6c77f758ec9e9828ccb..9d6b71e0fdd7304491a0c541e2b3abaa420a99aa 100644 (file)
--- 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.
  */