[libvirt] [PATCH v2 01/14] phyp: Resolve some file descriptor leaks
John Ferlan
jferlan at redhat.com
Thu Jan 10 19:21:02 UTC 2013
The phypUUIDTable_Push and phypUUIDTable_Pull leaked their file descriptors
on normal return. Each function had an unnecessary use of creating a buffer
to print conn->uri->user and needed a bit better flow control. I also noted
that the Read function had a cut-n-paste error from the write function on a
couple of VIR_WARN's.
The openSSHSession leaked the sock on the failure path. Additionally that
turns into the internal_socket in the phypOpen code. That was neither saved
nor closed on any path. So I used the connnection_data->sock field to save
the socket for eventual close. Of interest here is that phypExec used the
connection_data->sock field even though it had never been initialized.
---
src/phyp/phyp_driver.c | 114 ++++++++++++++++++-------------------------------
1 file changed, 41 insertions(+), 73 deletions(-)
diff --git a/src/phyp/phyp_driver.c b/src/phyp/phyp_driver.c
index f89871f..f6c2579 100644
--- a/src/phyp/phyp_driver.c
+++ b/src/phyp/phyp_driver.c
@@ -493,42 +493,30 @@ phypUUIDTable_Push(virConnectPtr conn)
ConnectionData *connection_data = conn->networkPrivateData;
LIBSSH2_SESSION *session = connection_data->session;
LIBSSH2_CHANNEL *channel = NULL;
- virBuffer username = VIR_BUFFER_INITIALIZER;
struct stat local_fileinfo;
char buffer[1024];
int rc = 0;
- FILE *fd = NULL;
+ FILE *f = NULL;
size_t nread, sent;
char *ptr;
char local_file[] = "./uuid_table";
char *remote_file = NULL;
+ int ret = -1;
- if (conn->uri->user != NULL) {
- virBufferAdd(&username, conn->uri->user, -1);
-
- if (virBufferError(&username)) {
- virBufferFreeAndReset(&username);
- virReportOOMError();
- goto err;
- }
- }
-
- if (virAsprintf
- (&remote_file, "/home/%s/libvirt_uuid_table",
- virBufferContentAndReset(&username))
- < 0) {
+ if (virAsprintf(&remote_file, "/home/%s/libvirt_uuid_table",
+ NULLSTR(conn->uri->user)) < 0) {
virReportOOMError();
- goto err;
+ goto cleanup;
}
if (stat(local_file, &local_fileinfo) == -1) {
VIR_WARN("Unable to stat local file.");
- goto err;
+ goto cleanup;
}
- if (!(fd = fopen(local_file, "rb"))) {
+ if (!(f = fopen(local_file, "rb"))) {
VIR_WARN("Unable to open local file.");
- goto err;
+ goto cleanup;
}
do {
@@ -539,18 +527,18 @@ phypUUIDTable_Push(virConnectPtr conn)
if ((!channel) && (libssh2_session_last_errno(session) !=
LIBSSH2_ERROR_EAGAIN))
- goto err;
+ goto cleanup;
} while (!channel);
do {
- nread = fread(buffer, 1, sizeof(buffer), fd);
+ nread = fread(buffer, 1, sizeof(buffer), f);
if (nread <= 0) {
- if (feof(fd)) {
+ if (feof(f)) {
/* end of file */
break;
} else {
VIR_ERROR(_("Failed to read from %s"), local_file);
- goto err;
+ goto cleanup;
}
}
ptr = buffer;
@@ -570,17 +558,9 @@ phypUUIDTable_Push(virConnectPtr conn)
} while (rc > 0 && sent < nread);
} while (1);
- if (channel) {
- libssh2_channel_send_eof(channel);
- libssh2_channel_wait_eof(channel);
- libssh2_channel_wait_closed(channel);
- libssh2_channel_free(channel);
- channel = NULL;
- }
- virBufferFreeAndReset(&username);
- return 0;
+ ret = 0;
-err:
+cleanup:
if (channel) {
libssh2_channel_send_eof(channel);
libssh2_channel_wait_eof(channel);
@@ -588,8 +568,8 @@ err:
libssh2_channel_free(channel);
channel = NULL;
}
- VIR_FORCE_FCLOSE(fd);
- return -1;
+ VIR_FORCE_FCLOSE(f);
+ return ret;
}
static int
@@ -665,7 +645,7 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
int id;
if ((fd = open(local_file, O_RDONLY)) == -1) {
- VIR_WARN("Unable to write information to local file.");
+ VIR_WARN("Unable to read information from local file.");
goto err;
}
@@ -682,13 +662,13 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
uuid_table->lpars[i]->id = id;
} else {
VIR_WARN
- ("Unable to read from information to local file.");
+ ("Unable to read from information from local file.");
goto err;
}
rc = read(fd, uuid_table->lpars[i]->uuid, VIR_UUID_BUFLEN);
if (rc != VIR_UUID_BUFLEN) {
- VIR_WARN("Unable to read information to local file.");
+ VIR_WARN("Unable to read information from local file.");
goto err;
}
}
@@ -709,34 +689,22 @@ phypUUIDTable_Pull(virConnectPtr conn)
ConnectionData *connection_data = conn->networkPrivateData;
LIBSSH2_SESSION *session = connection_data->session;
LIBSSH2_CHANNEL *channel = NULL;
- virBuffer username = VIR_BUFFER_INITIALIZER;
struct stat fileinfo;
char buffer[1024];
int rc = 0;
- int fd;
+ int fd = -1;
int got = 0;
int amount = 0;
int total = 0;
int sock = 0;
char local_file[] = "./uuid_table";
char *remote_file = NULL;
+ int ret = -1;
- if (conn->uri->user != NULL) {
- virBufferAdd(&username, conn->uri->user, -1);
-
- if (virBufferError(&username)) {
- virBufferFreeAndReset(&username);
- virReportOOMError();
- goto err;
- }
- }
-
- if (virAsprintf
- (&remote_file, "/home/%s/libvirt_uuid_table",
- virBufferContentAndReset(&username))
- < 0) {
+ if (virAsprintf(&remote_file, "/home/%s/libvirt_uuid_table",
+ NULLSTR(conn->uri->user)) < 0) {
virReportOOMError();
- goto err;
+ goto cleanup;
}
/* Trying to stat the remote file. */
@@ -746,12 +714,12 @@ phypUUIDTable_Pull(virConnectPtr conn)
if (!channel) {
if (libssh2_session_last_errno(session) !=
LIBSSH2_ERROR_EAGAIN) {
- goto err;
+ goto cleanup;
} else {
if (waitsocket(sock, session) < 0 && errno != EINTR) {
virReportSystemError(errno, "%s",
_("unable to wait on libssh2 socket"));
- goto err;
+ goto cleanup;
}
}
}
@@ -759,7 +727,7 @@ phypUUIDTable_Pull(virConnectPtr conn)
/* Creating a new data base based on remote file */
if ((fd = creat(local_file, 0755)) == -1)
- goto err;
+ goto cleanup;
/* Request a file via SCP */
while (got < fileinfo.st_size) {
@@ -790,7 +758,7 @@ phypUUIDTable_Pull(virConnectPtr conn)
if (waitsocket(sock, session) < 0 && errno != EINTR) {
virReportSystemError(errno, "%s",
_("unable to wait on libssh2 socket"));
- goto err;
+ goto cleanup;
}
continue;
}
@@ -799,20 +767,12 @@ phypUUIDTable_Pull(virConnectPtr conn)
if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno, _("Could not close %s"),
local_file);
- goto err;
+ goto cleanup;
}
- if (channel) {
- libssh2_channel_send_eof(channel);
- libssh2_channel_wait_eof(channel);
- libssh2_channel_wait_closed(channel);
- libssh2_channel_free(channel);
- channel = NULL;
- }
- virBufferFreeAndReset(&username);
- return 0;
+ ret = 0;
-err:
+cleanup:
if (channel) {
libssh2_channel_send_eof(channel);
libssh2_channel_wait_eof(channel);
@@ -820,7 +780,8 @@ err:
libssh2_channel_free(channel);
channel = NULL;
}
- return -1;
+ VIR_FORCE_CLOSE(fd);
+ return ret;
}
static int
@@ -985,7 +946,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
const char *hostname = conn->uri->server;
char *username = NULL;
char *password = NULL;
- int sock;
+ int sock = -1;
int rc;
struct addrinfo *ai = NULL, *cur;
struct addrinfo hints;
@@ -1135,6 +1096,7 @@ disconnect:
libssh2_session_disconnect(session, "Disconnecting...");
libssh2_session_free(session);
err:
+ VIR_FORCE_CLOSE(sock);
VIR_FREE(userhome);
VIR_FREE(pubkey);
VIR_FREE(pvtkey);
@@ -1191,6 +1153,7 @@ phypOpen(virConnectPtr conn,
virReportOOMError();
goto failure;
}
+ connection_data->sock = -1;
if (conn->uri->path) {
/* need to shift one byte in order to remove the first "/" of URI component */
@@ -1227,6 +1190,7 @@ phypOpen(virConnectPtr conn,
}
connection_data->session = session;
+ connection_data->sock = internal_socket;
uuid_table->nlpars = 0;
uuid_table->lpars = NULL;
@@ -1270,6 +1234,8 @@ failure:
libssh2_session_free(session);
}
+ if (connection_data)
+ VIR_FORCE_CLOSE(connection_data->sock);
VIR_FREE(connection_data);
return VIR_DRV_OPEN_ERROR;
@@ -1289,6 +1255,8 @@ phypClose(virConnectPtr conn)
phypUUIDTable_Free(phyp_driver->uuid_table);
VIR_FREE(phyp_driver->managed_system);
VIR_FREE(phyp_driver);
+
+ VIR_FORCE_CLOSE(connection_data->sock);
VIR_FREE(connection_data);
return 0;
}
--
1.7.11.7
More information about the libvir-list
mailing list