From 5ff384be086a82e05901f37e07d16ab2b585c4c8 Mon Sep 17 00:00:00 2001 From: Kulyatskaya Alexandra Date: Mon, 6 Oct 2025 23:19:56 +0300 Subject: [PATCH] semihosting: fix memory leak and double free 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 Reviewed-on: https://review.openocd.org/c/openocd/+/9196 Tested-by: jenkins Reviewed-by: Evgeniy Naydanov Reviewed-by: Antonio Borneo --- src/server/server.c | 22 +++++++++------------- src/server/server.h | 2 ++ src/target/semihosting_common.c | 12 ++++++------ 3 files changed, 17 insertions(+), 19 deletions(-) diff --git a/src/server/server.c b/src/server/server.c index 5f6bb584e..494fd9da3 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->service_dtor) + c->service_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->service_dtor = driver->service_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 ea1e94ec5..393dba769 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 (*service_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 (*service_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 ef96f064e..345e542c3 100644 --- a/src/target/semihosting_common.c +++ b/src/target/semihosting_common.c @@ -1800,13 +1800,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) @@ -1825,7 +1824,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, + .service_dtor_handler = semihosting_service_dtor_handler, + .connection_closed_handler = NULL, .keep_client_alive_handler = NULL, };