[PATCH]: 8037b53887 transport: validate the list of transport from the driver

[gerrit]

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/+/8676

-- gerrit

commit 8037b5388742ee97ac42de59550aa1fc36ebc4d6
Author: Antonio Borneo <borneo.anto...@gmail.com>
Date:   Sun Dec 22 23:59:19 2024 +0100

    transport: validate the list of transport from the driver
    
    Verify that the list contains only valid transports.
    
    While JTAG and SWD are the more permissive transports, the
    respective 'dapdirect' versions are slightly limited, and the
    respective 'hla' versions are even more limited.
    A driver should not provide two version of the same transport.
    Verify that only one JTAG and only one SWD transport is present.
    
    Change-Id: Iace2f881dd65fc763e81b33e6a7113961a7008af
    Signed-off-by: Antonio Borneo <borneo.anto...@gmail.com>

diff --git a/src/transport/transport.c b/src/transport/transport.c
index 9f70ef163a..76eb11cd7f 100644
--- a/src/transport/transport.c
+++ b/src/transport/transport.c
@@ -30,6 +30,7 @@
  * messaging and error handling.
  */
 
+#include <helper/bits.h>
 #include <helper/log.h>
 #include <helper/replacements.h>
 #include <transport/transport.h>
@@ -101,21 +102,33 @@ static int transport_select(struct command_context *ctx, 
const char *name)
  */
 int allow_transports(struct command_context *ctx, const unsigned char *vector)
 {
-       /* NOTE:  caller is required to provide only a list
-        * of *valid* transport names
-        *
-        * REVISIT should we validate that?  and insist there's
-        * at least one non-NULL element in that list?
-        *
-        * ... allow removals, e.g. external strapping prevents use
-        * of one transport; C code should be definitive about what
-        * can be used when all goes well.
-        */
        if (allowed_transports || session) {
                LOG_ERROR("Can't modify the set of allowed transports.");
                return ERROR_FAIL;
        }
 
+       /* validate the values in vector */
+       unsigned int mask = 0;
+       for (const unsigned char *v = vector; *v; v++) {
+               if (!is_transport_id_valid(*v)) {
+                       LOG_ERROR("Unknown transport ID %d", *v);
+                       return ERROR_FAIL;
+               }
+               mask |= BIT(*v);
+       }
+       if (((mask & BIT(TRANSPORT_JTAG)) && (mask & BIT(TRANSPORT_HLA_JTAG)))
+               || ((mask & BIT(TRANSPORT_JTAG)) && (mask & 
BIT(TRANSPORT_DAPDIRECT_JTAG)))
+               || ((mask & BIT(TRANSPORT_HLA_JTAG)) && (mask & 
BIT(TRANSPORT_DAPDIRECT_JTAG)))) {
+               LOG_ERROR("Multiple JTAG transports");
+               return ERROR_FAIL;
+       }
+       if (((mask & BIT(TRANSPORT_SWD)) && (mask & BIT(TRANSPORT_HLA_SWD)))
+               || ((mask & BIT(TRANSPORT_SWD)) && (mask & 
BIT(TRANSPORT_DAPDIRECT_SWD)))
+               || ((mask & BIT(TRANSPORT_HLA_SWD)) && (mask & 
BIT(TRANSPORT_DAPDIRECT_SWD)))) {
+               LOG_ERROR("Multiple SWD transports");
+               return ERROR_FAIL;
+       }
+
        allowed_transports = vector;
 
        /* autoselect if there's no choice ... */

--