target: fix memory leaks on target_create() fail
authorAntonio Borneo <borneo.antonio@gmail.com>
Sat, 11 Jul 2020 14:37:28 +0000 (16:37 +0200)
committerAntonio Borneo <borneo.antonio@gmail.com>
Sat, 8 Aug 2020 21:17:43 +0000 (22:17 +0100)
There are failure cases of target_create() that are not checked.
Plus, in case of failure the memory allocated in not properly
released before returning error.

Check all the possible failure in target_create().
Change current_target only when target is successfully created.
Add the new target to all_targets list only when target is
successfully created.
Release all the allocated memory before quit on failure.
Use malloc() instead of calloc() for target->type, because the
struct will be fully populated with memcpy().

Change-Id: Ib6f91cbb50c28878e7c73dc070b17b8d7d4e902f
Signed-off-by: Antonio Borneo <borneo.antonio@gmail.com>
Reviewed-on: http://openocd.zylin.com/5776
Tested-by: jenkins
src/target/target.c

index 5efa088b2d72d5c28294aa5348bee8e8918edaf1..4ef5ee19ef1bf005062d346619e0fa2b643e2dc0 100644 (file)
@@ -344,6 +344,15 @@ static int new_target_number(void)
        return x + 1;
 }
 
+static void append_to_list_all_targets(struct target *target)
+{
+       struct target **t = &all_targets;
+
+       while (*t)
+               t = &((*t)->next);
+       *t = target;
+}
+
 /* read a uint64_t from a buffer in target memory endianness */
 uint64_t target_buffer_get_u64(struct target *target, const uint8_t *buffer)
 {
@@ -5487,12 +5496,21 @@ static int target_create(Jim_GetOptInfo *goi)
 
        /* Create it */
        target = calloc(1, sizeof(struct target));
+       if (!target) {
+               LOG_ERROR("Out of memory");
+               return JIM_ERR;
+       }
+
        /* set target number */
        target->target_number = new_target_number();
-       cmd_ctx->current_target = target;
 
        /* allocate memory for each unique target type */
-       target->type = calloc(1, sizeof(struct target_type));
+       target->type = malloc(sizeof(struct target_type));
+       if (!target->type) {
+               LOG_ERROR("Out of memory");
+               free(target);
+               return JIM_ERR;
+       }
 
        memcpy(target->type, target_types[x], sizeof(struct target_type));
 
@@ -5521,6 +5539,12 @@ static int target_create(Jim_GetOptInfo *goi)
 
        /* initialize trace information */
        target->trace_info = calloc(1, sizeof(struct trace));
+       if (!target->trace_info) {
+               LOG_ERROR("Out of memory");
+               free(target->type);
+               free(target);
+               return JIM_ERR;
+       }
 
        target->dbgmsg          = NULL;
        target->dbg_msg_enabled = 0;
@@ -5554,7 +5578,9 @@ static int target_create(Jim_GetOptInfo *goi)
        }
 
        if (e != JIM_OK) {
+               rtos_destroy(target);
                free(target->gdb_port_override);
+               free(target->trace_info);
                free(target->type);
                free(target);
                return e;
@@ -5567,14 +5593,25 @@ static int target_create(Jim_GetOptInfo *goi)
 
        cp = Jim_GetString(new_cmd, NULL);
        target->cmd_name = strdup(cp);
+       if (!target->cmd_name) {
+               LOG_ERROR("Out of memory");
+               rtos_destroy(target);
+               free(target->gdb_port_override);
+               free(target->trace_info);
+               free(target->type);
+               free(target);
+               return JIM_ERR;
+       }
 
        if (target->type->target_create) {
                e = (*(target->type->target_create))(target, goi->interp);
                if (e != ERROR_OK) {
                        LOG_DEBUG("target_create failed");
+                       free(target->cmd_name);
+                       rtos_destroy(target);
                        free(target->gdb_port_override);
+                       free(target->trace_info);
                        free(target->type);
-                       free(target->cmd_name);
                        free(target);
                        return JIM_ERR;
                }
@@ -5587,15 +5624,6 @@ static int target_create(Jim_GetOptInfo *goi)
                        LOG_ERROR("unable to register '%s' commands", cp);
        }
 
-       /* append to end of list */
-       {
-               struct target **tpp;
-               tpp = &(all_targets);
-               while (*tpp)
-                       tpp = &((*tpp)->next);
-               *tpp = target;
-       }
-
        /* now - create the new target name command */
        const struct command_registration target_subcommands[] = {
                {
@@ -5617,14 +5645,27 @@ static int target_create(Jim_GetOptInfo *goi)
                COMMAND_REGISTRATION_DONE
        };
        e = register_commands(cmd_ctx, NULL, target_commands);
-       if (ERROR_OK != e)
+       if (e != ERROR_OK) {
+               if (target->type->deinit_target)
+                       target->type->deinit_target(target);
+               free(target->cmd_name);
+               rtos_destroy(target);
+               free(target->gdb_port_override);
+               free(target->trace_info);
+               free(target->type);
+               free(target);
                return JIM_ERR;
+       }
 
        struct command *c = command_find_in_context(cmd_ctx, cp);
        assert(c);
        command_set_handler_data(c, target);
 
-       return (ERROR_OK == e) ? JIM_OK : JIM_ERR;
+       /* append to end of list */
+       append_to_list_all_targets(target);
+
+       cmd_ctx->current_target = target;
+       return JIM_OK;
 }
 
 static int jim_target_current(Jim_Interp *interp, int argc, Jim_Obj *const *argv)