From 5b5ad96e89dad760c8254d47776efedf69c9ecc8 Mon Sep 17 00:00:00 2001 From: Francis Deslauriers Date: Thu, 22 Apr 2021 18:50:57 -0400 Subject: [PATCH] Fix: event-expr.c: use-after-free and NULL ptr deref in error path MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit use-after-free ============== The following functions are affected: `lttng_event_expr_app_specific_context_field_create()`, and `lttng_event_expr_array_field_element_create()`. In one error path we call `lttng_event_expr_destroy()` with the `&expr->parent` pointer (which is dynamically allocated) and this function then calls free() on that pointer. Right after that function call we return the pointer that was just freed. Fix that by adding a `ret_parent_expr` pointer that is set to NULL on the error path; Null pointer dereference ======================== The following functions are affected: `lttng_event_expr_app_specific_context_field_create()`, `lttng_event_expr_array_field_element_create()`, and `create_field_event_expr()`. We dereference a NULL pointer if the argument sanity check fails. Fix that by checking if `expr` is non-null before dereferencing it. Found with scan-build. Signed-off-by: Francis Deslauriers Signed-off-by: Jérémie Galarneau Change-Id: I5fdde462d7345d0dae7ecd2e4f46473a92cd11a9 --- src/lib/lttng-ctl/event-expr.c | 23 ++++++++++++++++++----- 1 file changed, 18 insertions(+), 5 deletions(-) diff --git a/src/lib/lttng-ctl/event-expr.c b/src/lib/lttng-ctl/event-expr.c index 273866ca3..126f0ca34 100644 --- a/src/lib/lttng-ctl/event-expr.c +++ b/src/lib/lttng-ctl/event-expr.c @@ -73,7 +73,10 @@ struct lttng_event_expr_field *create_field_event_expr( goto end; error: - lttng_event_expr_destroy(&expr->parent); + if (expr) { + lttng_event_expr_destroy(&expr->parent); + } + expr = NULL; end: return expr; @@ -117,6 +120,7 @@ struct lttng_event_expr *lttng_event_expr_app_specific_context_field_create( const char *provider_name, const char *type_name) { struct lttng_event_expr_app_specific_context_field *expr = NULL; + struct lttng_event_expr *ret_parent_expr; if (!type_name || !provider_name) { goto error; @@ -141,13 +145,17 @@ struct lttng_event_expr *lttng_event_expr_app_specific_context_field_create( goto error; } + ret_parent_expr = &expr->parent; goto end; error: - lttng_event_expr_destroy(&expr->parent); + if (expr) { + lttng_event_expr_destroy(&expr->parent); + } + ret_parent_expr = NULL; end: - return &expr->parent; + return ret_parent_expr; } struct lttng_event_expr *lttng_event_expr_array_field_element_create( @@ -155,6 +163,7 @@ struct lttng_event_expr *lttng_event_expr_array_field_element_create( unsigned int index) { struct lttng_event_expr_array_field_element *expr = NULL; + struct lttng_event_expr *ret_parent_expr; /* The parent array field expression must be an l-value */ if (!array_field_expr || @@ -173,13 +182,17 @@ struct lttng_event_expr *lttng_event_expr_array_field_element_create( expr->array_field_expr = array_field_expr; expr->index = index; + ret_parent_expr = &expr->parent; goto end; error: - lttng_event_expr_destroy(&expr->parent); + if (expr) { + lttng_event_expr_destroy(&expr->parent); + } + ret_parent_expr = NULL; end: - return &expr->parent; + return ret_parent_expr; } const char *lttng_event_expr_event_payload_field_get_name( -- 2.34.1