This is an automated email from Gerrit. "Alexandra Kulyatskaya <[email protected]>" just uploaded a new patch set to Gerrit, which you can find at https://review.openocd.org/c/openocd/+/9196
-- gerrit commit 9fbcc94b5353a38f306b3a307ea7e2068a1d59d3 Author: Kulyatskaya Alexandra <[email protected]> Date: Mon Oct 6 23:19:56 2025 +0300 [src/server]: segfaults on disabling of a dropped semihosting redirection YCAT-43681 Resolve two problems that occurred when working with semihosting service through multiple connection cycles (connect-disconnect-reconnect): 1) Double free: When the same service handles multiple connections sequentially, the same memory gets freed repeatedly, because function 'semihosting_service_connection_closed_handler()' incorrectly frees service->priv->name on every connection closure. 2) Memory leak: Function 'free_services()' misses service->priv->name cleanup for semihosting redirection. Memory remains allocated after service destruction. The solution introduces a new 'dtor()' field in the service structure that is called exactly once during free_service() execution. To reproduce the issue, you can do the following: 1. openocd -f target.cfg -c init -c 'arm semihosting enable' -c 'arm semihosting_redirect tcp 4445' # in another terminal 2. nc localhost 4445 3. Ctr+C 4. nc localhost 4445 5. Ctr+C Change-Id: I0dc8021cc3e21c5af619c71a1821a1afe9bffe78 Signed-off-by: Kulyatskaya Alexandra <[email protected]> diff --git a/src/server/server.c b/src/server/server.c index 5f6bb584e5..3ae91c2309 100644 --- a/src/server/server.c +++ b/src/server/server.c @@ -161,7 +161,8 @@ static int remove_connection(struct service *service, struct connection *connect /* find connection */ while ((c = *p)) { if (c->fd == connection->fd) { - service->connection_closed(c); + if (service->connection_closed) + service->connection_closed(c); if (service->type == CONNECTION_TCP) close_socket(c->fd); else if (service->type == CONNECTION_PIPE) { @@ -190,8 +191,13 @@ static int remove_connection(struct service *service, struct connection *connect static void free_service(struct service *c) { + if (c->type == CONNECTION_PIPE && c->fd != -1) + close(c->fd); + if (c->dtor) + c->dtor(c); free(c->name); free(c->port); + free(c->priv); free(c); } @@ -214,6 +220,7 @@ int add_service(const struct service_driver *driver, const char *port, c->input = driver->input_handler; c->connection_closed = driver->connection_closed_handler; c->keep_client_alive = driver->keep_client_alive_handler; + c->dtor = driver->dtor_handler; c->priv = priv; c->next = NULL; long portnumber; @@ -390,18 +397,7 @@ static int remove_services(void) struct service *next = c->next; remove_connections(c); - - free(c->name); - - if (c->type == CONNECTION_PIPE) { - if (c->fd != -1) - close(c->fd); - } - free(c->port); - free(c->priv); - /* delete service */ - free(c); - + free_service(c); /* remember the last service for unlinking */ c = next; } diff --git a/src/server/server.h b/src/server/server.h index ea1e94ec5f..a9ee4fdb1a 100644 --- a/src/server/server.h +++ b/src/server/server.h @@ -58,6 +58,7 @@ struct service_driver { int (*new_connection_handler)(struct connection *connection); /** callback to handle incoming data */ int (*input_handler)(struct connection *connection); + void (*dtor_handler)(struct service *service); /** callback to tear down the connection */ int (*connection_closed_handler)(struct connection *connection); /** called periodically to send keep-alive messages on the connection */ @@ -76,6 +77,7 @@ struct service { int (*new_connection_during_keep_alive)(struct connection *connection); int (*new_connection)(struct connection *connection); int (*input)(struct connection *connection); + void (*dtor)(struct service *service); int (*connection_closed)(struct connection *connection); void (*keep_client_alive)(struct connection *connection); void *priv; diff --git a/src/target/semihosting_common.c b/src/target/semihosting_common.c index 5f8ab1082c..15070a88b9 100644 --- a/src/target/semihosting_common.c +++ b/src/target/semihosting_common.c @@ -1798,13 +1798,12 @@ static int semihosting_service_input_handler(struct connection *connection) return ERROR_OK; } -static int semihosting_service_connection_closed_handler(struct connection *connection) +static void semihosting_service_dtor_handler(struct service *service) { - struct semihosting_tcp_service *service = connection->service->priv; - if (service) - free(service->name); + struct semihosting_tcp_service *service_priv = service->priv; + if (service_priv) + free(service_priv->name); - return ERROR_OK; } static void semihosting_tcp_close_cnx(struct semihosting *semihosting) @@ -1823,7 +1822,8 @@ static const struct service_driver semihosting_service_driver = { .new_connection_during_keep_alive_handler = NULL, .new_connection_handler = semihosting_service_new_connection_handler, .input_handler = semihosting_service_input_handler, - .connection_closed_handler = semihosting_service_connection_closed_handler, + .dtor_handler = semihosting_service_dtor_handler, + .connection_closed_handler = NULL, .keep_client_alive_handler = NULL, }; --
