This is an automated email from Gerrit.
"Antonio Borneo <borneo.anto...@gmail.com>" just uploaded a new patch set to
Gerrit, which you can find at https://review.openocd.org/c/openocd/+/6852
-- gerrit
commit 0a377bd7c446f9fc0a58cc410c946188a1dcee4d
Author: Antonio Borneo <borneo.anto...@gmail.com>
Date: Sat Feb 19 16:16:31 2022 +0100
gdb_server: fix double free
Commit 6541233aa78d ("Combine register lists of smp targets.")
unconditionally assigns the output pointers of the function
smp_reg_list_noread(), even if the function fails and returns
error.
This causes a double free from the caller, that has assigned NULL
to the pointers to simplify the error handling.
Use local variables in smp_reg_list_noread() and assign the output
pointers only on success.
Change-Id: Ic0fd2f26520566cf322f0190780e15637c01cfae
Fixes: 6541233aa78d ("Combine register lists of smp targets.")
Reported-by: Michele Bisogno <michele.bisogno...@renesas.com>
Signed-off-by: Antonio Borneo <borneo.anto...@gmail.com>
diff --git a/src/server/gdb_server.c b/src/server/gdb_server.c
index 95720e561..f8a1aac83 100644
--- a/src/server/gdb_server.c
+++ b/src/server/gdb_server.c
@@ -2272,12 +2272,12 @@ static int smp_reg_list_noread(struct target *target,
combined_list_size, REG_CLASS_ALL);
unsigned int combined_allocated = 256;
- *combined_list = malloc(combined_allocated * sizeof(struct reg *));
- if (*combined_list == NULL) {
+ struct reg **local_list = malloc(combined_allocated * sizeof(struct reg
*));
+ if (!local_list) {
LOG_ERROR("malloc(%zu) failed", combined_allocated *
sizeof(struct reg *));
return ERROR_FAIL;
}
- *combined_list_size = 0;
+ unsigned int local_list_size = 0;
struct target_list *head;
foreach_smp_target(head, target->smp_targets) {
@@ -2286,7 +2286,7 @@ static int smp_reg_list_noread(struct target *target,
int result = target_get_gdb_reg_list_noread(head->target,
®_list,
®_list_size, reg_class);
if (result != ERROR_OK) {
- free(*combined_list);
+ free(local_list);
return result;
}
for (int i = 0; i < reg_list_size; i++) {
@@ -2296,8 +2296,8 @@ static int smp_reg_list_noread(struct target *target,
/* Nested loop makes this O(n^2), but this
entire function with
* 5 RISC-V targets takes just 2ms on my
computer. Fast enough
* for me. */
- for (int j = 0; j < *combined_list_size; j++) {
- struct reg *b = (*combined_list)[j];
+ for (unsigned int j = 0; j < local_list_size;
j++) {
+ struct reg *b = local_list[j];
if (!strcmp(a->name, b->name)) {
found = true;
if (a->size != b->size) {
@@ -2305,7 +2305,7 @@ static int smp_reg_list_noread(struct target *target,
"target, but %d bits on another target.",
a->name, a->size, b->size);
free(reg_list);
- free(*combined_list);
+ free(local_list);
return ERROR_FAIL;
}
break;
@@ -2313,16 +2313,16 @@ static int smp_reg_list_noread(struct target *target,
}
if (!found) {
LOG_DEBUG("[%s] %s not found in
combined list", target_name(target), a->name);
- if (*combined_list_size >= (int)
combined_allocated) {
+ if (local_list_size >=
combined_allocated) {
combined_allocated *= 2;
- *combined_list =
realloc(*combined_list, combined_allocated * sizeof(struct reg *));
- if (*combined_list == NULL) {
+ local_list =
realloc(local_list, combined_allocated * sizeof(struct reg *));
+ if (!local_list) {
LOG_ERROR("realloc(%zu)
failed", combined_allocated * sizeof(struct reg *));
return ERROR_FAIL;
}
}
- (*combined_list)[*combined_list_size] =
a;
- (*combined_list_size)++;
+ local_list[local_list_size] = a;
+ local_list_size++;
}
}
}
@@ -2336,12 +2336,12 @@ static int smp_reg_list_noread(struct target *target,
int result = target_get_gdb_reg_list_noread(head->target,
®_list,
®_list_size, reg_class);
if (result != ERROR_OK) {
- free(*combined_list);
+ free(local_list);
return result;
}
- for (int i = 0; i < *combined_list_size; i++) {
+ for (unsigned int i = 0; i < local_list_size; i++) {
bool found = false;
- struct reg *a = (*combined_list)[i];
+ struct reg *a = local_list[i];
for (int j = 0; j < reg_list_size; j++) {
struct reg *b = reg_list[j];
if (b->exist && !strcmp(a->name, b->name)) {
@@ -2358,6 +2358,8 @@ static int smp_reg_list_noread(struct target *target,
free(reg_list);
}
+ *combined_list = local_list;
+ *combined_list_size = local_list_size;
return ERROR_OK;
}
--
