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>

1 files changed, 16 insertions, 6 deletions

diff --git a/io/channel-command.c b/io/channel-command.c
index a220fe8..a9c67aa 100644
--- a/io/channel-command.c
+++ b/io/channel-command.c
@@ -66,7 +66,7 @@ qio_channel_command_new_spawn(const char *const argv[],
 
     if (stdinnull || stdoutnull) {
         devnull = open("/dev/null", O_RDWR);
-        if (!devnull) {
+        if (devnull < 0) {
             error_setg_errno(errp, errno,
                              "Unable to open /dev/null");
             goto error;
@@ -98,6 +98,9 @@ qio_channel_command_new_spawn(const char *const argv[],
             close(stdoutfd[0]);
             close(stdoutfd[1]);
         }
+        if (devnull != -1) {
+            close(devnull);
+        }
 
         execv(argv[0], (char * const *)argv);
         _exit(1);
@@ -117,6 +120,9 @@ qio_channel_command_new_spawn(const char *const argv[],
     return ioc;
 
  error:
+    if (devnull != -1) {
+        close(devnull);
+    }
     if (stdinfd[0] != -1) {
         close(stdinfd[0]);
     }
@@ -202,12 +208,12 @@ static void qio_channel_command_finalize(Object *obj)
     QIOChannelCommand *ioc = QIO_CHANNEL_COMMAND(obj);
     if (ioc->readfd != -1) {
         close(ioc->readfd);
-        ioc->readfd = -1;
     }
-    if (ioc->writefd != -1) {
+    if (ioc->writefd != -1 &&
+        ioc->writefd != ioc->readfd) {
         close(ioc->writefd);
-        ioc->writefd = -1;
     }
+    ioc->writefd = ioc->readfd = -1;
     if (ioc->pid > 0) {
 #ifndef WIN32
         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
      * gives a better chance of clean shutdown
      */
-    if (close(cioc->writefd) < 0) {
+    if (cioc->readfd != -1 &&
+        close(cioc->readfd) < 0) {
         rv = -1;
     }
-    if (close(cioc->readfd) < 0) {
+    if (cioc->writefd != -1 &&
+        cioc->writefd != cioc->readfd &&
+        close(cioc->writefd) < 0) {
         rv = -1;
     }
+    cioc->writefd = cioc->readfd = -1;
 #ifndef WIN32
     if (qio_channel_command_abort(cioc, errp) < 0) {
         return -1;