This is an automated email from Gerrit.

"Evgeniy Naydanov <[email protected]>" just uploaded a new patch set to Gerrit, 
which you can find at https://review.openocd.org/c/openocd/+/9729

-- gerrit

commit c5849714fdfbcb8503ed52c1aa925fbd9b43c267
Author: Evgeniy Naydanov <[email protected]>
Date:   Tue Sep 23 13:06:09 2025 +0300

    target: improve SMP target list management in `target smp`
    
    Before the patch updating the SMP target list could result in a
    use-after-free:
    ```
    > valgrind openocd \
            -c 'adapter driver dummy' \
            -c 'proc jtag_init {} {}' \
            -c 'jtag newtap tap0 cpu -irlen 2' \
            -c 'target create t0 testee -chain-position tap0.cpu' \
            -c 'target create t1 testee -chain-position tap0.cpu' \
            -c 'target smp t0 t1' \
            -c 'target smp t0' \
            -c shutdown
    ...
    == Invalid read of size 8
    ==    at 0x4274D16: breakpoint_remove_all_internal (breakpoints.c:330)
    ...
    ==    by 0x410EA21: target_quit (target.c:2261)
    ...
    ==  Address 0x5380248 is 136 bytes inside a block of size 312 free'd
    ...
    ==    by 0x410EA21: target_quit (target.c:2261)
    ...
    == Invalid read of size 8
    ==    at 0x42750B3: watchpoint_remove_all_internal (breakpoints.c:415)
    ...
    ==    by 0x410EA21: target_quit (target.c:2261)
    ...
    ==  Address 0x5380250 is 144 bytes inside a block of size 312 free'd
    ...
    ==    by 0x410EA21: target_quit (target.c:2261)
    ...
    == Invalid write of size 4
    ==    at 0x410E880: target_destroy (target.c:2221)
    ...
    ==  Address 0x53802bc is 252 bytes inside a block of size 312 free'd
    ...
    ==    by 0x410EA21: target_quit (target.c:2261)
    ...
    == ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
    ```
    
    Change-Id: I886e8bb4cc0433ec045bfd57f78ceef843d8f774
    Signed-off-by: Evgeniy Naydanov <[email protected]>

diff --git a/src/target/target.c b/src/target/target.c
index d68e9afb19..f3fac5f742 100644
--- a/src/target/target.c
+++ b/src/target/target.c
@@ -2209,6 +2209,22 @@ uint32_t target_get_working_area_avail(struct target 
*target)
        return max_size;
 }
 
+static void free_smp_target_list(struct list_head *smp_targets)
+{
+       assert(smp_targets);
+       if (smp_targets == &empty_smp_targets)
+               return;
+
+       struct target_list *head, *tmp;
+       list_for_each_entry_safe(head, tmp, smp_targets, lh) {
+               list_del(&head->lh);
+               head->target->smp = 0;
+               head->target->smp_targets = &empty_smp_targets;
+               free(head);
+       }
+       free(smp_targets);
+}
+
 static void target_destroy(struct target *target)
 {
        breakpoint_remove_all(target);
@@ -2232,19 +2248,7 @@ static void target_destroy(struct target *target)
 
        target_free_all_working_areas(target);
 
-       /* release the targets SMP list */
-       if (target->smp) {
-               struct target_list *head, *tmp;
-
-               list_for_each_entry_safe(head, tmp, target->smp_targets, lh) {
-                       list_del(&head->lh);
-                       head->target->smp = 0;
-                       free(head);
-               }
-               if (target->smp_targets != &empty_smp_targets)
-                       free(target->smp_targets);
-               target->smp = 0;
-       }
+       free_smp_target_list(target->smp_targets);
 
        rtos_destroy(target);
 
@@ -6006,8 +6010,11 @@ COMMAND_HANDLER(handle_target_smp)
                if (new)
                        list_add_tail(&new->lh, lh);
        }
-       /*  now parse the list of cpu and put the target in smp mode*/
        struct target_list *curr;
+       foreach_smp_target(curr, lh) {
+               struct target *target = curr->target;
+               free_smp_target_list(target->smp_targets);
+       }
        foreach_smp_target(curr, lh) {
                struct target *target = curr->target;
                target->smp = smp_group;
@@ -6020,6 +6027,9 @@ COMMAND_HANDLER(handle_target_smp)
        if (retval == ERROR_OK && rtos_target)
                retval = rtos_smp_init(rtos_target);
 
+       if (retval != ERROR_OK)
+               free_smp_target_list(lh);
+
        return retval;
 }
 
diff --git a/testing/tcl_commands/Makefile.am b/testing/tcl_commands/Makefile.am
index fe8d150707..f579bcaf80 100644
--- a/testing/tcl_commands/Makefile.am
+++ b/testing/tcl_commands/Makefile.am
@@ -5,7 +5,8 @@ TESTS =
 if DUMMY
 TESTS += \
        test-target-create-command.cfg \
-       test-target-configure-cget-command.cfg
+       test-target-configure-cget-command.cfg \
+       test-target-smp-command.cfg
 endif
 
 EXTRA_DIST = utils.tcl $(TESTS)
diff --git a/testing/tcl_commands/test-target-smp-command.cfg 
b/testing/tcl_commands/test-target-smp-command.cfg
new file mode 100644
index 0000000000..e7cd7506f4
--- /dev/null
+++ b/testing/tcl_commands/test-target-smp-command.cfg
@@ -0,0 +1,15 @@
+# SPDX-License-Identifier: GPL-2.0-or-later
+
+adapter driver dummy
+jtag newtap tap0 cpu -irlen 2
+jtag newtap tap1 cpu -irlen 2
+target create tgt0 testee -chain-position tap0.cpu
+target create tgt1 testee -chain-position tap1.cpu
+
+target smp tgt0 tgt1
+
+target smp tgt0
+
+target smp tgt0 tgt0
+
+shutdown

-- 

Reply via email to