forked from auracaster/openocd
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.kulyatskaya@syntacore.com>
Reviewed-on: https://review.openocd.org/c/openocd/+/9196
Tested-by: jenkins
Reviewed-by: Evgeniy Naydanov <eugnay@gmail.com>
Reviewed-by: Antonio Borneo <borneo.antonio@gmail.com>
This commit is contained in:
committed by
Antonio Borneo
parent
55e9160509
commit
5ff384be08
@@ -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;
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
@@ -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,
|
||||
};
|
||||
|
||||
|
||||
Reference in New Issue
Block a user