io: some fixes to handling of /dev/null when running commands

The /dev/null file handle was leaked in a couple of places.
There is also the possibility that both readfd and writefd
point to the same /dev/null file handle, so care must be
taken not to close the same file handle twice.

Reviewed-by: Paolo Bonzini <pbonzini@redhat.com>
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
This commit is contained in:
Daniel P. Berrange 2016-01-11 13:02:16 +00:00
parent 0c0a55b229
commit e155494cf0

View File

@ -66,7 +66,7 @@ qio_channel_command_new_spawn(const char *const argv[],
if (stdinnull || stdoutnull) { if (stdinnull || stdoutnull) {
devnull = open("/dev/null", O_RDWR); devnull = open("/dev/null", O_RDWR);
if (!devnull) { if (devnull < 0) {
error_setg_errno(errp, errno, error_setg_errno(errp, errno,
"Unable to open /dev/null"); "Unable to open /dev/null");
goto error; goto error;
@ -98,6 +98,9 @@ qio_channel_command_new_spawn(const char *const argv[],
close(stdoutfd[0]); close(stdoutfd[0]);
close(stdoutfd[1]); close(stdoutfd[1]);
} }
if (devnull != -1) {
close(devnull);
}
execv(argv[0], (char * const *)argv); execv(argv[0], (char * const *)argv);
_exit(1); _exit(1);
@ -117,6 +120,9 @@ qio_channel_command_new_spawn(const char *const argv[],
return ioc; return ioc;
error: error:
if (devnull != -1) {
close(devnull);
}
if (stdinfd[0] != -1) { if (stdinfd[0] != -1) {
close(stdinfd[0]); close(stdinfd[0]);
} }
@ -202,12 +208,12 @@ static void qio_channel_command_finalize(Object *obj)
QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj); QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj);
if (ioc->readfd != -1) { if (ioc->readfd != -1) {
close(ioc->readfd); close(ioc->readfd);
ioc->readfd = -1;
} }
if (ioc->writefd != -1) { if (ioc->writefd != -1 &&
ioc->writefd != ioc->readfd) {
close(ioc->writefd); close(ioc->writefd);
ioc->writefd = -1;
} }
ioc->writefd = ioc->readfd = -1;
if (ioc->pid > 0) { if (ioc->pid > 0) {
#ifndef WIN32 #ifndef WIN32
qio_channel_command_abort(ioc, NULL); qio_channel_command_abort(ioc, NULL);
@ -299,12 +305,16 @@ static int qio_channel_command_close(QIOChannel *ioc,
/* We close FDs before killing, because that /* We close FDs before killing, because that
* gives a better chance of clean shutdown * gives a better chance of clean shutdown
*/ */
if (close(cioc->writefd) < 0) { if (cioc->readfd != -1 &&
close(cioc->readfd) < 0) {
rv = -1; rv = -1;
} }
if (close(cioc->readfd) < 0) { if (cioc->writefd != -1 &&
cioc->writefd != cioc->readfd &&
close(cioc->writefd) < 0) {
rv = -1; rv = -1;
} }
cioc->writefd = cioc->readfd = -1;
#ifndef WIN32 #ifndef WIN32
if (qio_channel_command_abort(cioc, errp) < 0) { if (qio_channel_command_abort(cioc, errp) < 0) {
return -1; return -1;