phyp: Resolve some file descriptor leaks

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.
This commit is contained in:
John Ferlan 2013-01-10 14:21:02 -05:00 committed by Eric Blake
parent daa886b635
commit b17409674e

View File

@ -1,5 +1,5 @@
/* /*
* Copyright (C) 2010-2012 Red Hat, Inc. * Copyright (C) 2010-2013 Red Hat, Inc.
* Copyright IBM Corp. 2009 * Copyright IBM Corp. 2009
* *
* phyp_driver.c: ssh layer to access Power Hypervisors * phyp_driver.c: ssh layer to access Power Hypervisors
@ -493,42 +493,30 @@ phypUUIDTable_Push(virConnectPtr conn)
ConnectionData *connection_data = conn->networkPrivateData; ConnectionData *connection_data = conn->networkPrivateData;
LIBSSH2_SESSION *session = connection_data->session; LIBSSH2_SESSION *session = connection_data->session;
LIBSSH2_CHANNEL *channel = NULL; LIBSSH2_CHANNEL *channel = NULL;
virBuffer username = VIR_BUFFER_INITIALIZER;
struct stat local_fileinfo; struct stat local_fileinfo;
char buffer[1024]; char buffer[1024];
int rc = 0; int rc = 0;
FILE *fd = NULL; FILE *f = NULL;
size_t nread, sent; size_t nread, sent;
char *ptr; char *ptr;
char local_file[] = "./uuid_table"; char local_file[] = "./uuid_table";
char *remote_file = NULL; char *remote_file = NULL;
int ret = -1;
if (conn->uri->user != NULL) { if (virAsprintf(&remote_file, "/home/%s/libvirt_uuid_table",
virBufferAdd(&username, conn->uri->user, -1); NULLSTR(conn->uri->user)) < 0) {
if (virBufferError(&username)) {
virBufferFreeAndReset(&username);
virReportOOMError();
goto err;
}
}
if (virAsprintf
(&remote_file, "/home/%s/libvirt_uuid_table",
virBufferContentAndReset(&username))
< 0) {
virReportOOMError(); virReportOOMError();
goto err; goto cleanup;
} }
if (stat(local_file, &local_fileinfo) == -1) { if (stat(local_file, &local_fileinfo) == -1) {
VIR_WARN("Unable to stat local file."); 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."); VIR_WARN("Unable to open local file.");
goto err; goto cleanup;
} }
do { do {
@ -539,18 +527,18 @@ phypUUIDTable_Push(virConnectPtr conn)
if ((!channel) && (libssh2_session_last_errno(session) != if ((!channel) && (libssh2_session_last_errno(session) !=
LIBSSH2_ERROR_EAGAIN)) LIBSSH2_ERROR_EAGAIN))
goto err; goto cleanup;
} while (!channel); } while (!channel);
do { do {
nread = fread(buffer, 1, sizeof(buffer), fd); nread = fread(buffer, 1, sizeof(buffer), f);
if (nread <= 0) { if (nread <= 0) {
if (feof(fd)) { if (feof(f)) {
/* end of file */ /* end of file */
break; break;
} else { } else {
VIR_ERROR(_("Failed to read from %s"), local_file); VIR_ERROR(_("Failed to read from %s"), local_file);
goto err; goto cleanup;
} }
} }
ptr = buffer; ptr = buffer;
@ -570,17 +558,9 @@ phypUUIDTable_Push(virConnectPtr conn)
} while (rc > 0 && sent < nread); } while (rc > 0 && sent < nread);
} while (1); } while (1);
if (channel) { ret = 0;
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;
err: cleanup:
if (channel) { if (channel) {
libssh2_channel_send_eof(channel); libssh2_channel_send_eof(channel);
libssh2_channel_wait_eof(channel); libssh2_channel_wait_eof(channel);
@ -588,8 +568,8 @@ err:
libssh2_channel_free(channel); libssh2_channel_free(channel);
channel = NULL; channel = NULL;
} }
VIR_FORCE_FCLOSE(fd); VIR_FORCE_FCLOSE(f);
return -1; return ret;
} }
static int static int
@ -665,7 +645,7 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
int id; int id;
if ((fd = open(local_file, O_RDONLY)) == -1) { 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; goto err;
} }
@ -682,13 +662,13 @@ phypUUIDTable_ReadFile(virConnectPtr conn)
uuid_table->lpars[i]->id = id; uuid_table->lpars[i]->id = id;
} else { } else {
VIR_WARN VIR_WARN
("Unable to read from information to local file."); ("Unable to read from information from local file.");
goto err; goto err;
} }
rc = read(fd, uuid_table->lpars[i]->uuid, VIR_UUID_BUFLEN); rc = read(fd, uuid_table->lpars[i]->uuid, VIR_UUID_BUFLEN);
if (rc != 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; goto err;
} }
} }
@ -709,34 +689,22 @@ phypUUIDTable_Pull(virConnectPtr conn)
ConnectionData *connection_data = conn->networkPrivateData; ConnectionData *connection_data = conn->networkPrivateData;
LIBSSH2_SESSION *session = connection_data->session; LIBSSH2_SESSION *session = connection_data->session;
LIBSSH2_CHANNEL *channel = NULL; LIBSSH2_CHANNEL *channel = NULL;
virBuffer username = VIR_BUFFER_INITIALIZER;
struct stat fileinfo; struct stat fileinfo;
char buffer[1024]; char buffer[1024];
int rc = 0; int rc = 0;
int fd; int fd = -1;
int got = 0; int got = 0;
int amount = 0; int amount = 0;
int total = 0; int total = 0;
int sock = 0; int sock = 0;
char local_file[] = "./uuid_table"; char local_file[] = "./uuid_table";
char *remote_file = NULL; char *remote_file = NULL;
int ret = -1;
if (conn->uri->user != NULL) { if (virAsprintf(&remote_file, "/home/%s/libvirt_uuid_table",
virBufferAdd(&username, conn->uri->user, -1); NULLSTR(conn->uri->user)) < 0) {
if (virBufferError(&username)) {
virBufferFreeAndReset(&username);
virReportOOMError();
goto err;
}
}
if (virAsprintf
(&remote_file, "/home/%s/libvirt_uuid_table",
virBufferContentAndReset(&username))
< 0) {
virReportOOMError(); virReportOOMError();
goto err; goto cleanup;
} }
/* Trying to stat the remote file. */ /* Trying to stat the remote file. */
@ -746,12 +714,12 @@ phypUUIDTable_Pull(virConnectPtr conn)
if (!channel) { if (!channel) {
if (libssh2_session_last_errno(session) != if (libssh2_session_last_errno(session) !=
LIBSSH2_ERROR_EAGAIN) { LIBSSH2_ERROR_EAGAIN) {
goto err; goto cleanup;
} else { } else {
if (waitsocket(sock, session) < 0 && errno != EINTR) { if (waitsocket(sock, session) < 0 && errno != EINTR) {
virReportSystemError(errno, "%s", virReportSystemError(errno, "%s",
_("unable to wait on libssh2 socket")); _("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 */ /* Creating a new data base based on remote file */
if ((fd = creat(local_file, 0755)) == -1) if ((fd = creat(local_file, 0755)) == -1)
goto err; goto cleanup;
/* Request a file via SCP */ /* Request a file via SCP */
while (got < fileinfo.st_size) { while (got < fileinfo.st_size) {
@ -790,7 +758,7 @@ phypUUIDTable_Pull(virConnectPtr conn)
if (waitsocket(sock, session) < 0 && errno != EINTR) { if (waitsocket(sock, session) < 0 && errno != EINTR) {
virReportSystemError(errno, "%s", virReportSystemError(errno, "%s",
_("unable to wait on libssh2 socket")); _("unable to wait on libssh2 socket"));
goto err; goto cleanup;
} }
continue; continue;
} }
@ -799,9 +767,12 @@ phypUUIDTable_Pull(virConnectPtr conn)
if (VIR_CLOSE(fd) < 0) { if (VIR_CLOSE(fd) < 0) {
virReportSystemError(errno, _("Could not close %s"), virReportSystemError(errno, _("Could not close %s"),
local_file); local_file);
goto err; goto cleanup;
} }
ret = 0;
cleanup:
if (channel) { if (channel) {
libssh2_channel_send_eof(channel); libssh2_channel_send_eof(channel);
libssh2_channel_wait_eof(channel); libssh2_channel_wait_eof(channel);
@ -809,18 +780,8 @@ phypUUIDTable_Pull(virConnectPtr conn)
libssh2_channel_free(channel); libssh2_channel_free(channel);
channel = NULL; channel = NULL;
} }
virBufferFreeAndReset(&username); VIR_FORCE_CLOSE(fd);
return 0; return ret;
err:
if (channel) {
libssh2_channel_send_eof(channel);
libssh2_channel_wait_eof(channel);
libssh2_channel_wait_closed(channel);
libssh2_channel_free(channel);
channel = NULL;
}
return -1;
} }
static int static int
@ -985,7 +946,7 @@ openSSHSession(virConnectPtr conn, virConnectAuthPtr auth,
const char *hostname = conn->uri->server; const char *hostname = conn->uri->server;
char *username = NULL; char *username = NULL;
char *password = NULL; char *password = NULL;
int sock; int sock = -1;
int rc; int rc;
struct addrinfo *ai = NULL, *cur; struct addrinfo *ai = NULL, *cur;
struct addrinfo hints; struct addrinfo hints;
@ -1135,6 +1096,7 @@ disconnect:
libssh2_session_disconnect(session, "Disconnecting..."); libssh2_session_disconnect(session, "Disconnecting...");
libssh2_session_free(session); libssh2_session_free(session);
err: err:
VIR_FORCE_CLOSE(sock);
VIR_FREE(userhome); VIR_FREE(userhome);
VIR_FREE(pubkey); VIR_FREE(pubkey);
VIR_FREE(pvtkey); VIR_FREE(pvtkey);
@ -1191,6 +1153,7 @@ phypOpen(virConnectPtr conn,
virReportOOMError(); virReportOOMError();
goto failure; goto failure;
} }
connection_data->sock = -1;
if (conn->uri->path) { if (conn->uri->path) {
/* need to shift one byte in order to remove the first "/" of URI component */ /* 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->session = session;
connection_data->sock = internal_socket;
uuid_table->nlpars = 0; uuid_table->nlpars = 0;
uuid_table->lpars = NULL; uuid_table->lpars = NULL;
@ -1270,6 +1234,8 @@ failure:
libssh2_session_free(session); libssh2_session_free(session);
} }
if (connection_data)
VIR_FORCE_CLOSE(connection_data->sock);
VIR_FREE(connection_data); VIR_FREE(connection_data);
return VIR_DRV_OPEN_ERROR; return VIR_DRV_OPEN_ERROR;
@ -1289,6 +1255,8 @@ phypClose(virConnectPtr conn)
phypUUIDTable_Free(phyp_driver->uuid_table); phypUUIDTable_Free(phyp_driver->uuid_table);
VIR_FREE(phyp_driver->managed_system); VIR_FREE(phyp_driver->managed_system);
VIR_FREE(phyp_driver); VIR_FREE(phyp_driver);
VIR_FORCE_CLOSE(connection_data->sock);
VIR_FREE(connection_data); VIR_FREE(connection_data);
return 0; return 0;
} }