Skip to content

Commit

Permalink
fix leak on one of the error branches of _dd_command_exec
Browse files Browse the repository at this point in the history
  • Loading branch information
cataphract committed Dec 20, 2024
1 parent cf7254e commit 41d91c4
Showing 1 changed file with 16 additions and 11 deletions.
27 changes: 16 additions & 11 deletions appsec/src/extension/commands_helpers.c
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ typedef struct _dd_imsg {
mpack_node_t root;
} dd_imsg;

// iif this returns success, _imsg_destroy must be called
// if and only if this returns success, _imsg_destroy must be called
static dd_result ATTR_WARN_UNUSED _imsg_recv(
dd_imsg *nonnull imsg, dd_conn *nonnull conn);

static inline ATTR_WARN_UNUSED mpack_error_t _imsg_destroy(
dd_imsg *nonnull imsg);
static void _imsg_cleanup(dd_imsg *nullable *imsg);

static dd_result _dd_command_exec(dd_conn *nonnull conn,
const dd_command_spec *nonnull spec, void *unspecnull ctx)
Expand Down Expand Up @@ -98,20 +99,21 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
return res;
}

// automatic cleanup of imsg on error branches
// set to NULL before calling _imsg_destroy
__attribute__((cleanup(_imsg_cleanup))) dd_imsg *nullable destroy_imsg =
&imsg;

mpack_node_t first_response = mpack_node_array_at(imsg.root, 0);
mpack_error_t err = mpack_node_error(first_response);
if (err != mpack_ok) {
mlog(dd_log_error, "Array of responses could not be retrieved - %s",
mpack_error_to_string(err));
// NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
err = _imsg_destroy(&imsg);
return dd_error;
}
if (mpack_node_type(first_response) != mpack_type_array) {
mlog(dd_log_error, "Invalid response. Expected array but got %s",
mpack_type_to_string(mpack_node_type(first_response)));
// NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
err = _imsg_destroy(&imsg);
return dd_error;
}
mpack_node_t first_message = mpack_node_array_at(first_response, 1);
Expand All @@ -127,16 +129,12 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
if (err != mpack_ok) {
mlog(dd_log_error, "Response type could not be retrieved - %s",
mpack_error_to_string(err));
// NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
err = _imsg_destroy(&imsg);
return dd_error;
}
if (mpack_node_type(type) != mpack_type_str) {
mlog(dd_log_error,
"Unexpected type field. Expected string but got %s",
mpack_type_to_string(mpack_node_type(type)));
// NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
err = _imsg_destroy(&imsg);
return dd_error;
}
if (dd_mpack_node_lstr_eq(type, "config_features")) {
Expand All @@ -147,8 +145,6 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
mlog(dd_log_debug,
"Received message for command %.*s unexpected: %.*s\n", NAME_L,
(int)mpack_node_strlen(type), mpack_node_str(type));
// NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores)
err = _imsg_destroy(&imsg);
return dd_error;
}

Expand All @@ -157,6 +153,7 @@ static dd_result _dd_command_exec(dd_conn *nonnull conn,
err = imsg.root.tree->error;
_dump_in_msg(err == mpack_ok ? dd_log_trace : dd_log_debug, imsg._data,
imsg._size);
destroy_imsg = NULL;
err = _imsg_destroy(&imsg);
if (err != mpack_ok) {
mlog(dd_log_warning,
Expand Down Expand Up @@ -281,6 +278,14 @@ static inline ATTR_WARN_UNUSED mpack_error_t _imsg_destroy(
return mpack_tree_destroy(&imsg->_tree);
}

static void _imsg_cleanup(dd_imsg *nullable *imsg)
{
dd_imsg **imsg_c = (dd_imsg * nullable * nonnull) imsg;
if (*imsg_c) {
UNUSED(_imsg_destroy(*imsg_c));
}
}

/* Baked response */

static void _add_appsec_span_data_frag(mpack_node_t node);
Expand Down

0 comments on commit 41d91c4

Please sign in to comment.