]> git.ipfire.org Git - thirdparty/strongswan.git/commitdiff
utils: Don't use directory enumerator to close open FDs in closefrom()
authorTobias Brunner <tobias@strongswan.org>
Tue, 4 Aug 2015 12:43:26 +0000 (14:43 +0200)
committerTobias Brunner <tobias@strongswan.org>
Mon, 17 Aug 2015 09:19:32 +0000 (11:19 +0200)
Calling malloc() after fork() is potentially unsafe, so we should avoid
it if possible.  opendir() will still require an allocation but that's
less than the variant using the enumerator wrapper, thus, decreasing
the conflict potential.  This way we can also avoid closing the
FD for the enumerated directory itself.

References #990.

src/libstrongswan/utils/utils.c

index 9b516accd411b9593f3a4e603746d8423819ec63..55eab8e14a17de790081a7e7cde9d07c190d3ea1 100644 (file)
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2008-2014 Tobias Brunner
+ * Copyright (C) 2008-2015 Tobias Brunner
  * Copyright (C) 2005-2008 Martin Willi
  * Hochschule fuer Technik Rapperswil
  *
 
 #include "utils.h"
 
+#include <sys/types.h>
 #include <unistd.h>
 #include <limits.h>
+#include <ctype.h>
 #ifndef WIN32
 # include <signal.h>
 #endif
 
+#ifndef HAVE_CLOSEFROM
+# include <dirent.h>
+#endif
+
 #include <library.h>
 #include <collections/enumerator.h>
 
+#define FD_DIR "/proc/self/fd"
+
 #ifdef WIN32
 
 #include <threading/mutex.h>
@@ -110,43 +118,47 @@ void wait_sigint()
 /**
  * Described in header.
  */
-void closefrom(int lowfd)
+void closefrom(int low_fd)
 {
-       char fd_dir[PATH_MAX];
-       int maxfd, fd, len;
-
-       /* try to close only open file descriptors on Linux... */
-       len = snprintf(fd_dir, sizeof(fd_dir), "/proc/%u/fd", getpid());
-       if (len > 0 && len < sizeof(fd_dir) && access(fd_dir, F_OK) == 0)
+       DIR *dir;
+       struct dirent *entry;
+       int max_fd, dir_fd, fd;
+
+       /* try to close only open file descriptors on Linux... This is potentially
+        * unsafe when called after fork() in multi-threaded applications.  In
+        * particular opendir() will require an allocation.  So it depends on how
+        * the malloc() implementation handles such situations */
+       dir = opendir(FD_DIR);
+       if (dir)
        {
-               enumerator_t *enumerator = enumerator_create_directory(fd_dir);
-               if (enumerator)
+               dir_fd = dirfd(dir);
+               while ((entry = readdir(dir)))
                {
-                       char *rel;
-                       while (enumerator->enumerate(enumerator, &rel, NULL, NULL))
+                       if (!isdigit(entry->d_name[0]))
+                       {
+                               continue;
+                       }
+                       fd = atoi(entry->d_name);
+                       if (fd != dir_fd && fd >= low_fd)
                        {
-                               fd = atoi(rel);
-                               if (fd >= lowfd)
-                               {
-                                       close(fd);
-                               }
+                               close(fd);
                        }
-                       enumerator->destroy(enumerator);
-                       return;
                }
+               closedir(dir);
+               return;
        }
 
        /* ...fall back to closing all fds otherwise */
 #ifdef WIN32
-       maxfd = _getmaxstdio();
+       max_fd = _getmaxstdio();
 #else
-       maxfd = (int)sysconf(_SC_OPEN_MAX);
+       max_fd = (int)sysconf(_SC_OPEN_MAX);
 #endif
-       if (maxfd < 0)
+       if (max_fd < 0)
        {
-               maxfd = 256;
+               max_fd = 256;
        }
-       for (fd = lowfd; fd < maxfd; fd++)
+       for (fd = low_fd; fd < max_fd; fd++)
        {
                close(fd);
        }