[PATCH] Set the FD_CLOEXEC flag on the runtime's file descriptors

[PATCH] Set the FD_CLOEXEC flag on the runtime's file descriptors

[Andreas Rottmann]

This is supposed to prevent Guile to leak internal file descriptors
across an exec* system call.  The Guile user has still to take care of
setting the CLOEXEC flag on all ports (e.g., using `port-for-each').

Linux's LVM tools are a nice test case for this, as they emit a warning
line for each leaked FD when invoked.

[PATCH] Set the FD_CLOEXEC flag on the runtime's file descriptors

[Andreas Rottmann]

* libguile/_scm.h (scm_set_fd_cloexec): New convenience macro for
  setting the FD_CLOEXEC flah on platforms that support it; on other
  platforms it's a no-op.

* libguile/objcodes.c (scm_load_objcode): Mark the objectcode's FD as
  close-on-exec.
* libguile/scmsigs.c (start_signal_delivery_thread): Mark both ends of
  the signal delivery pipe as close-on-exec.
* libguile/threads.c (guilify_self_1): Likewise for the thread's
  sleep_pipe.
---
 libguile/_scm.h     |   12 ++++++++++++
 libguile/objcodes.c |    2 ++
 libguile/scmsigs.c  |    4 +++-
 libguile/threads.c  |    3 +++
 4 files changed, 20 insertions(+), 1 deletions(-)

diff --git a/libguile/_scm.h b/libguile/_scm.h
index 2842130..8eff18f 100644
--- a/libguile/_scm.h
+++ b/libguile/_scm.h
@@ -62,6 +62,7 @@
 #endif
 
 #include <errno.h>
+#include <fcntl.h>
 #include <verify.h>
 #include <alignof.h>
 #include "libguile/__scm.h"
@@ -132,6 +133,17 @@
 # define SCM_SYSCALL(line) line;
 #endif /* ndef SCM_SYSCALL */
 
+#if defined(HAVE_FCNTL) && defined(FD_CLOEXEC)
+#  define scm_set_fd_cloexec(fd)                        \
+  do  {                                                 \
+    int old_flags = fcntl (fd, F_GETFD);                \
+    if (old_flags >= 0)                                 \
+      fcntl (fd, F_SETFD, FD_CLOEXEC | old_flags);      \
+  } while (0)
+#else
+#  define scm_set_fd_cloexec(fd) fd
+#endif
+
 \f
 
 #ifndef min
diff --git a/libguile/objcodes.c b/libguile/objcodes.c
index 448bada..d6a38fc 100644
--- a/libguile/objcodes.c
+++ b/libguile/objcodes.c
@@ -299,6 +299,8 @@ SCM_DEFINE (scm_load_objcode, "load-objcode", 1, 0, 0,
   free (c_file);
   if (fd < 0) SCM_SYSERROR;
 
+  scm_set_fd_cloexec (fd);
+
   return make_objcode_from_file (fd);
 }
 #undef FUNC_NAME
diff --git a/libguile/scmsigs.c b/libguile/scmsigs.c
index 699a6de..e84390a 100644
--- a/libguile/scmsigs.c
+++ b/libguile/scmsigs.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2004, 2006, 2007, 2008, 2009 Free Software Foundation, Inc.
+/* Copyright (C) 1995,1996,1997,1998,1999,2000,2001, 2002, 2004, 2006, 2007, 2008, 2009, 2011 Free Software Foundation, Inc.
  *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
@@ -190,6 +190,8 @@ start_signal_delivery_thread (void)
 
   if (pipe (signal_pipe) != 0)
     scm_syserror (NULL);
+  scm_set_fd_cloexec (signal_pipe[0]);
+  scm_set_fd_cloexec (signal_pipe[1]);
   signal_thread = scm_spawn_thread (signal_delivery_thread, NULL,
 				    scm_handle_by_message,
 				    "signal delivery thread");
diff --git a/libguile/threads.c b/libguile/threads.c
index f49696b..5d986e9 100644
--- a/libguile/threads.c
+++ b/libguile/threads.c
@@ -526,6 +526,9 @@ guilify_self_1 (struct GC_stack_base *base)
        currently have type `void'.  */
     abort ();
 
+  scm_set_fd_cloexec (t.sleep_pipe[0]);
+  scm_set_fd_cloexec (t.sleep_pipe[1]);
+
   scm_i_pthread_mutex_init (&t.admin_mutex, NULL);
   t.current_mark_stack_ptr = NULL;
   t.current_mark_stack_limit = NULL;
-- 
1.7.5.1

Re: [PATCH] Set the FD_CLOEXEC flag on the runtime's file descriptors

[Andy Wingo]

Hi,

Sorry for the long delay here.

On Sun 08 May 2011 00:29, Andreas Rottmann <a.rottmann@gmx.at> writes:

> * libguile/_scm.h (scm_set_fd_cloexec): New convenience macro for
>   setting the FD_CLOEXEC flah on platforms that support it; on other
>   platforms it's a no-op.

This isn't the right fix, I don't think.  I think the right thing is to
use open with O_CLOEXEC in objcodes.c (possibly with the open gnulib
module) and pipe2 with O_CLOEXEC in scmsigs.c and threads.c (possibly
with the pipe2 gnulib module).  I've done that in HEAD now.  Thanks for
the motivation!

Andy
-- 
http://wingolog.org/