Merge remote-tracking branch 'mdroth/qga-pull-1-14-2013' into staging

* mdroth/qga-pull-1-14-2013:
  qemu-ga: Handle errors uniformely in ga_channel_open()
  qemu-ga: Plug fd leak on ga_channel_open() error paths
  qemu-ga: Plug fd leak on ga_channel_listen_accept() error path
  qemu-ga: Plug file descriptor leak on ga_open_pidfile() error path
  qemu-ga: Drop pointless lseek() from ga_open_pidfile()
  qemu-ga: Document intentional fall through in channel_event_cb()
  qemu-ga: add ga_open_logfile()
  qemu-ga: ga_open_pidfile(): use qemu_open()

Signed-off-by: Anthony Liguori <aliguori@us.ibm.com>

2 files changed, 29 insertions, 8 deletions

diff --git a/qga/channel-posix.c b/qga/channel-posix.c
index ca9e4aaaf9..e65dda3822 100644
--- a/qga/channel-posix.c
+++ b/qga/channel-posix.c
@@ -46,6 +46,7 @@ static gboolean ga_channel_listen_accept(GIOChannel *channel,
     ret = ga_channel_client_add(c, client_fd);
     if (ret) {
         g_warning("error setting up connection");
+        close(client_fd);
         goto out;
     }
     accepted = true;
@@ -140,19 +141,21 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod
                            );
         if (fd == -1) {
             g_critical("error opening channel: %s", strerror(errno));
-            exit(EXIT_FAILURE);
+            return false;
         }
 #ifdef CONFIG_SOLARIS
         ret = ioctl(fd, I_SETSIG, S_OUTPUT | S_INPUT | S_HIPRI);
         if (ret == -1) {
             g_critical("error setting event mask for channel: %s",
                        strerror(errno));
-            exit(EXIT_FAILURE);
+            close(fd);
+            return false;
         }
 #endif
         ret = ga_channel_client_add(c, fd);
         if (ret) {
             g_critical("error adding channel to main loop");
+            close(fd);
             return false;
         }
         break;
@@ -162,7 +165,7 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod
         int fd = qemu_open(path, O_RDWR | O_NOCTTY | O_NONBLOCK);
         if (fd == -1) {
             g_critical("error opening channel: %s", strerror(errno));
-            exit(EXIT_FAILURE);
+            return false;
         }
         tcgetattr(fd, &tio);
         /* set up serial port for non-canonical, dumb byte streaming */
@@ -182,7 +185,9 @@ static gboolean ga_channel_open(GAChannel *c, const gchar *path, GAChannelMethod
         tcsetattr(fd, TCSANOW, &tio);
         ret = ga_channel_client_add(c, fd);
         if (ret) {
-            g_error("error adding channel to main loop");
+            g_critical("error adding channel to main loop");
+            close(fd);
+            return false;
         }
         break;
     }
diff --git a/qga/main.c b/qga/main.c
index a9b968c507..db281a508b 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -261,13 +261,26 @@ void ga_set_response_delimited(GAState *s)
     s->delimit_response = true;
 }
 
+static FILE *ga_open_logfile(const char *logfile)
+{
+    FILE *f;
+
+    f = fopen(logfile, "a");
+    if (!f) {
+        return NULL;
+    }
+
+    qemu_set_cloexec(fileno(f));
+    return f;
+}
+
 #ifndef _WIN32
 static bool ga_open_pidfile(const char *pidfile)
 {
     int pidfd;
     char pidstr[32];
 
-    pidfd = open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
+    pidfd = qemu_open(pidfile, O_CREAT|O_WRONLY, S_IRUSR|S_IWUSR);
     if (pidfd == -1 || lockf(pidfd, F_TLOCK, 0)) {
         g_critical("Cannot lock pid file, %s", strerror(errno));
         if (pidfd != -1) {
@@ -276,7 +289,7 @@ static bool ga_open_pidfile(const char *pidfile)
         return false;
     }
 
-    if (ftruncate(pidfd, 0) || lseek(pidfd, 0, SEEK_SET)) {
+    if (ftruncate(pidfd, 0)) {
         g_critical("Failed to truncate pid file");
         goto fail;
     }
@@ -286,10 +299,12 @@ static bool ga_open_pidfile(const char *pidfile)
         goto fail;
     }
 
+    /* keep pidfile open & locked forever */
     return true;
 
 fail:
     unlink(pidfile);
+    close(pidfd);
     return false;
 }
 #else /* _WIN32 */
@@ -402,7 +417,7 @@ void ga_unset_frozen(GAState *s)
      * in a frozen state at start up, do it now
      */
     if (s->deferred_options.log_filepath) {
-        s->log_file = fopen(s->deferred_options.log_filepath, "a");
+        s->log_file = ga_open_logfile(s->deferred_options.log_filepath);
         if (!s->log_file) {
             s->log_file = stderr;
         }
@@ -605,6 +620,7 @@ static gboolean channel_event_cb(GIOCondition condition, gpointer data)
         if (!s->virtio) {
             return false;
         }
+        /* fall through */
     case G_IO_STATUS_AGAIN:
         /* virtio causes us to spin here when no process is attached to
          * host-side chardev. sleep a bit to mitigate this
@@ -884,7 +900,7 @@ int main(int argc, char **argv)
             become_daemon(pid_filepath);
         }
         if (log_filepath) {
-            FILE *log_file = fopen(log_filepath, "a");
+            FILE *log_file = ga_open_logfile(log_filepath);
             if (!log_file) {
                 g_critical("unable to open specified log file: %s",
                            strerror(errno));