|
From: openocd-gerrit <ope...@us...> - 2025-11-22 19:25:40
|
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "Main OpenOCD repository".
The branch, master has been updated
via 5ff384be086a82e05901f37e07d16ab2b585c4c8 (commit)
from 55e916050981af472c1749b8cbb0e4940ba26273 (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
commit 5ff384be086a82e05901f37e07d16ab2b585c4c8
Author: Kulyatskaya Alexandra <a.k...@sy...>
Date: Mon Oct 6 23:19:56 2025 +0300
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 <a.k...@sy...>
Reviewed-on: https://review.openocd.org/c/openocd/+/9196
Tested-by: jenkins
Reviewed-by: Evgeniy Naydanov <eu...@gm...>
Reviewed-by: Antonio Borneo <bor...@gm...>
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,
};
-----------------------------------------------------------------------
Summary of changes:
src/server/server.c | 22 +++++++++-------------
src/server/server.h | 2 ++
src/target/semihosting_common.c | 12 ++++++------
3 files changed, 17 insertions(+), 19 deletions(-)
hooks/post-receive
--
Main OpenOCD repository
|