libssh2: fix freeing of resources in disconnect
ssh's disconnect assumed that the session to the server could be shut
down successfully during disconnect. When this failed, e.g. timed out,
memory was leaked.
Closes #16656
diff --git a/lib/vssh/libssh2.c b/lib/vssh/libssh2.c
index 0a88ec0..b5e2ac7 100644
--- a/lib/vssh/libssh2.c
+++ b/lib/vssh/libssh2.c
@@ -106,7 +106,8 @@
static CURLcode ssh_setup_connection(struct Curl_easy *data,
struct connectdata *conn);
static void ssh_attach(struct Curl_easy *data, struct connectdata *conn);
-
+static int sshc_cleanup(struct ssh_conn *sshc, struct Curl_easy *data,
+ bool block);
/*
* SCP protocol handler.
*/
@@ -2862,66 +2863,12 @@
break;
case SSH_SESSION_FREE:
- if(sshc->kh) {
- libssh2_knownhost_free(sshc->kh);
- sshc->kh = NULL;
- }
-
- if(sshc->ssh_agent) {
- rc = libssh2_agent_disconnect(sshc->ssh_agent);
- if(rc == LIBSSH2_ERROR_EAGAIN) {
- break;
- }
- if(rc < 0) {
- char *err_msg = NULL;
- (void)libssh2_session_last_error(sshc->ssh_session,
- &err_msg, NULL, 0);
- infof(data, "Failed to disconnect from libssh2 agent: %d %s",
- rc, err_msg);
- }
- libssh2_agent_free(sshc->ssh_agent);
- sshc->ssh_agent = NULL;
-
- /* NB: there is no need to free identities, they are part of internal
- agent stuff */
- sshc->sshagent_identity = NULL;
- sshc->sshagent_prev_identity = NULL;
- }
-
- if(sshc->ssh_session) {
- rc = libssh2_session_free(sshc->ssh_session);
- if(rc == LIBSSH2_ERROR_EAGAIN) {
- break;
- }
- if(rc < 0) {
- char *err_msg = NULL;
- (void)libssh2_session_last_error(sshc->ssh_session,
- &err_msg, NULL, 0);
- infof(data, "Failed to free libssh2 session: %d %s", rc, err_msg);
- }
- sshc->ssh_session = NULL;
- }
-
- /* worst-case scenario cleanup */
-
- DEBUGASSERT(sshc->ssh_session == NULL);
- DEBUGASSERT(sshc->ssh_channel == NULL);
- DEBUGASSERT(sshc->sftp_session == NULL);
- DEBUGASSERT(sshc->sftp_handle == NULL);
- DEBUGASSERT(sshc->kh == NULL);
- DEBUGASSERT(sshc->ssh_agent == NULL);
-
- Curl_safefree(sshc->rsa_pub);
- Curl_safefree(sshc->rsa);
- Curl_safefree(sshc->quote_path1);
- Curl_safefree(sshc->quote_path2);
- Curl_safefree(sshc->homedir);
-
+ rc = sshc_cleanup(sshc, data, FALSE);
+ if(rc == LIBSSH2_ERROR_EAGAIN)
+ break;
/* the code we are about to return */
result = sshc->actualcode;
-
memset(sshc, 0, sizeof(struct ssh_conn));
-
connclose(conn, "SSH session free");
sshc->state = SSH_SESSION_FREE; /* current */
sshc->nextstate = SSH_NO_STATE;
@@ -3376,6 +3323,69 @@
return result;
}
+static int sshc_cleanup(struct ssh_conn *sshc, struct Curl_easy *data,
+ bool block)
+{
+ int rc;
+
+ if(sshc->kh) {
+ libssh2_knownhost_free(sshc->kh);
+ sshc->kh = NULL;
+ }
+
+ if(sshc->ssh_agent) {
+ rc = libssh2_agent_disconnect(sshc->ssh_agent);
+ if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) {
+ return rc;
+ }
+ if(rc < 0) {
+ char *err_msg = NULL;
+ (void)libssh2_session_last_error(sshc->ssh_session,
+ &err_msg, NULL, 0);
+ infof(data, "Failed to disconnect from libssh2 agent: %d %s",
+ rc, err_msg);
+ }
+ libssh2_agent_free(sshc->ssh_agent);
+ sshc->ssh_agent = NULL;
+
+ /* NB: there is no need to free identities, they are part of internal
+ agent stuff */
+ sshc->sshagent_identity = NULL;
+ sshc->sshagent_prev_identity = NULL;
+ }
+
+ if(sshc->ssh_session) {
+ rc = libssh2_session_free(sshc->ssh_session);
+ if(!block && (rc == LIBSSH2_ERROR_EAGAIN)) {
+ return rc;
+ }
+ if(rc < 0) {
+ char *err_msg = NULL;
+ (void)libssh2_session_last_error(sshc->ssh_session,
+ &err_msg, NULL, 0);
+ infof(data, "Failed to free libssh2 session: %d %s", rc, err_msg);
+ }
+ sshc->ssh_session = NULL;
+ }
+
+ /* worst-case scenario cleanup */
+ DEBUGASSERT(sshc->ssh_session == NULL);
+ DEBUGASSERT(sshc->ssh_channel == NULL);
+ DEBUGASSERT(sshc->sftp_session == NULL);
+ DEBUGASSERT(sshc->sftp_handle == NULL);
+ DEBUGASSERT(sshc->kh == NULL);
+ DEBUGASSERT(sshc->ssh_agent == NULL);
+
+ Curl_safefree(sshc->rsa_pub);
+ Curl_safefree(sshc->rsa);
+ Curl_safefree(sshc->quote_path1);
+ Curl_safefree(sshc->quote_path2);
+ Curl_safefree(sshc->homedir);
+
+ return 0;
+}
+
+
/* BLOCKING, but the function is using the state machine so the only reason
this is still blocking is that the multi interface code has no support for
disconnecting operations that takes a while */
@@ -3393,6 +3403,7 @@
result = ssh_block_statemach(data, conn, TRUE);
}
+ sshc_cleanup(sshc, data, TRUE);
return result;
}
@@ -3549,6 +3560,7 @@
}
DEBUGF(infof(data, "SSH DISCONNECT is done"));
+ sshc_cleanup(sshc, data, TRUE);
return result;
