[Qemu-devel] [PATCH 0/2] qemu-ga: don't leak fds to exec()ed processes

[Qemu-devel] [PATCH 0/2] qemu-ga: don't leak fds to exec()ed processes

[Luiz Capitulino]

Please, check individual patches for details.

Luiz Capitulino (2):
  qemu-ga: ga_open_pidfile(): use qemu_open()
  qemu-ga: add ga_open_logfile()

 qga/main.c | 19 ++++++++++++++++---
 1 file changed, 16 insertions(+), 3 deletions(-)

-- 
1.8.0

[Qemu-devel] [PATCH 1/2] qemu-ga: ga_open_pidfile(): use qemu_open()

[Luiz Capitulino]

This ensures that O_CLOEXEC is passed to open(), this way the
pid file fd is not leaked to executed processes.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qga/main.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/qga/main.c b/qga/main.c
index ba5fa1c..e4245cc 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -242,7 +242,7 @@ 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) {
-- 
1.8.0

Re: [Qemu-devel] [PATCH 1/2] qemu-ga: ga_open_pidfile(): use qemu_open()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 1001 bytes --]

On 01/08/2013 02:26 PM, Luiz Capitulino wrote:
> This ensures that O_CLOEXEC is passed to open(), this way the
> pid file fd is not leaked to executed processes.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  qga/main.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

> 
> diff --git a/qga/main.c b/qga/main.c
> index ba5fa1c..e4245cc 100644
> --- a/qga/main.c
> +++ b/qga/main.c
> @@ -242,7 +242,7 @@ 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) {
> 

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

[Qemu-devel] [PATCH 2/2] qemu-ga: add ga_open_logfile()

[Luiz Capitulino]

This function sets O_CLOEXEC on the log file fd so that it isn't
leaked to executed processes.

Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
---
 qga/main.c | 17 +++++++++++++++--
 1 file changed, 15 insertions(+), 2 deletions(-)

diff --git a/qga/main.c b/qga/main.c
index e4245cc..15be74f 100644
--- a/qga/main.c
+++ b/qga/main.c
@@ -236,6 +236,19 @@ 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)
 {
@@ -377,7 +390,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;
         }
@@ -838,7 +851,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));
-- 
1.8.0

Re: [Qemu-devel] [PATCH 2/2] qemu-ga: add ga_open_logfile()

[Eric Blake]

[-- Attachment #1: Type: text/plain, Size: 468 bytes --]

On 01/08/2013 02:26 PM, Luiz Capitulino wrote:
> This function sets O_CLOEXEC on the log file fd so that it isn't
> leaked to executed processes.
> 
> Signed-off-by: Luiz Capitulino <lcapitulino@redhat.com>
> ---
>  qga/main.c | 17 +++++++++++++++--
>  1 file changed, 15 insertions(+), 2 deletions(-)

Reviewed-by: Eric Blake <eblake@redhat.com>

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 619 bytes --]

Re: [Qemu-devel] [PATCH 0/2] qemu-ga: don't leak fds to exec()ed processes

[Amos Kong]

On Tue, Jan 08, 2013 at 07:26:24PM -0200, Luiz Capitulino wrote:
> Please, check individual patches for details.
> 
> Luiz Capitulino (2):
>   qemu-ga: ga_open_pidfile(): use qemu_open()
>   qemu-ga: add ga_open_logfile()
 
Acked-by: Amos Kong <akong@redhat.com>
Tested-by: Amos Kong <akong@redhat.com>

>  qga/main.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> -- 
> 1.8.0

Re: [Qemu-devel] [PATCH 0/2] qemu-ga: don't leak fds to exec()ed processes

[mdroth]

On Tue, Jan 08, 2013 at 07:26:24PM -0200, Luiz Capitulino wrote:
> Please, check individual patches for details.
> 
> Luiz Capitulino (2):
>   qemu-ga: ga_open_pidfile(): use qemu_open()
>   qemu-ga: add ga_open_logfile()

Thanks, applied to qga branch.

> 
>  qga/main.c | 19 ++++++++++++++++---
>  1 file changed, 16 insertions(+), 3 deletions(-)
> 
> -- 
> 1.8.0
>