From f3dbee891d188edbcd505dd5ea19c03f499c92b7 Mon Sep 17 00:00:00 2001 From: Scott K Logan Date: Sun, 19 Jan 2025 21:48:30 -0600 Subject: [PATCH] Fix socket handle leak for outgoing TCP connections If a proxy client sends a TCP_OPEN message while the proxy already has an open connection, the proxy will leak the handle to the original connection to process the new one. After testing this scenario on the Java implementation, it appears to send a TCP_CLOSE message to the client for the original connection and then immediately open a new connection. It's hard to tell if this is what the EchoLink client wants us to do, but taking the same behavior as the Java proxy is certainly better than leaking the handle like we were doing, so that's what this change does. --- src/proxy_conn.c | 64 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 45 insertions(+), 19 deletions(-) diff --git a/src/proxy_conn.c b/src/proxy_conn.c index 8476133..da68886 100644 --- a/src/proxy_conn.c +++ b/src/proxy_conn.c @@ -291,6 +291,16 @@ static int process_tcp_open_message(struct proxy_conn_handle *pc, */ static int send_tcp_close(struct proxy_conn_handle *pc); +/*! + * @brief Send a ::PROXY_MSG_TYPE_TCP_STATUS message to the client + * + * @param[in,out] pc Target proxy client connection instance + * @param[in] status Status to report to the client + * + * @returns 0 on success, negative ERRNO value on failure + */ +static int send_tcp_status(struct proxy_conn_handle *pc, uint32_t status); + static void forwarder_control(struct worker_handle *wh) { struct proxy_conn_handle *pc = wh->func_ctx; @@ -706,8 +716,6 @@ static int process_tcp_open_message(struct proxy_conn_handle *pc, const struct proxy_msg *msg) { struct proxy_conn_priv *priv = pc->priv; - uint8_t status_buf[sizeof(struct proxy_msg) + 4] = { 0 }; - struct proxy_msg *status_msg = (struct proxy_msg *)status_buf; const uint8_t *addr_sep = (const uint8_t *)&msg->address; char addr[16] = ""; int ret; @@ -725,6 +733,14 @@ static int process_tcp_open_message(struct proxy_conn_handle *pc, return -EINVAL; } + /* Disconnect any existing connection */ + conn_close(&priv->conn_tcp); + ret = worker_wait_idle(&priv->worker_tcp); + if (ret < 0) { + send_tcp_status(pc, ret); + return ret; + } + /* Attempt to connect */ ret = conn_connect(&priv->conn_tcp, (const char *)addr, "5200"); if (ret < 0) { @@ -738,44 +754,54 @@ static int process_tcp_open_message(struct proxy_conn_handle *pc, conn_close(&priv->conn_tcp); } - status_msg->type = PROXY_MSG_TYPE_TCP_STATUS; - status_msg->size = 4; + return send_tcp_status(pc, ret); +} - /* Unless we can figure out what the client is expecting here, the - * best we can do is a "non-zero" value to indicate failure. - */ - memcpy(status_buf + sizeof(*status_msg), &ret, 4); +static int send_tcp_close(struct proxy_conn_handle *pc) +{ + struct proxy_conn_priv *priv = pc->priv; + struct proxy_msg message = { 0 }; + int ret; + + message.type = PROXY_MSG_TYPE_TCP_CLOSE; + message.size = 0; proxy_log(pc->ph, LOG_LEVEL_DEBUG, - "Sending TCP_STATUS message (%d) to client '%s'\n", - ret, priv->callsign); + "Sending TCP_CLOSE message to client '%s'\n", priv->callsign); mutex_lock(&priv->mutex_client_send); - ret = conn_send(priv->conn_client, status_buf, - sizeof(*status_msg) + status_msg->size); + ret = conn_send(priv->conn_client, (uint8_t *)&message, + sizeof(struct proxy_msg)); mutex_unlock(&priv->mutex_client_send); return ret; } -static int send_tcp_close(struct proxy_conn_handle *pc) +static int send_tcp_status(struct proxy_conn_handle *pc, uint32_t status) { struct proxy_conn_priv *priv = pc->priv; - struct proxy_msg message = { 0 }; + uint8_t status_buf[sizeof(struct proxy_msg) + 4] = { 0 }; + struct proxy_msg *status_msg = (struct proxy_msg *)status_buf; int ret; - message.type = PROXY_MSG_TYPE_TCP_CLOSE; - message.size = 0; + status_msg->type = PROXY_MSG_TYPE_TCP_STATUS; + status_msg->size = 4; + + /* Unless we can figure out what the client is expecting here, the + * best we can do is a "non-zero" value to indicate failure. + */ + memcpy(status_buf + sizeof(*status_msg), &status, 4); proxy_log(pc->ph, LOG_LEVEL_DEBUG, - "Sending TCP_CLOSE message to client '%s'\n", priv->callsign); + "Sending TCP_STATUS message (%d) to client '%s'\n", + status, priv->callsign); mutex_lock(&priv->mutex_client_send); - ret = conn_send(priv->conn_client, (uint8_t *)&message, - sizeof(struct proxy_msg)); + ret = conn_send(priv->conn_client, status_buf, + sizeof(struct proxy_msg) + status_msg->size); mutex_unlock(&priv->mutex_client_send);