[prev in list] [next in list] [prev in thread] [next in thread] 

List:       tar-bug
Subject:    [Bug-tar] fdopendir failure on MacOS X with GNU tar 1.25
From:       Paul Eggert <eggert () cs ! ucla ! edu>
Date:       2010-11-08 6:12:09
Message-ID: 4CD794B9.2030503 () cs ! ucla ! edu
[Download RAW message or body]

(This is following up on recent bug reports sent to bug-tar:

<http://lists.gnu.org/archive/html/bug-tar/2010-10/msg00084.html>
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00000.html>
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00038.html>.

but it appears to be a gnulib bug so I'm CC'ing this to bug-gnulib.)

On 11/07/2010 09:49 AM, tsteven4 wrote:

> I attached the output of "sudo dtruss -n tar -f -a > &
> tar125_dtruss.txt" while running "make check TESTSUITEFLAGS=37".

Thanks for sending that.  In looking through the dtruss output I see a
bug in gnulib's fdopendir.c implementation: when fdopendir(N) is
invoked, and N is the maximum openable file descriptor, and there are
some unopened file descriptors less than N, then fdopendir first uses
'dup' to consume these unopened file descriptors, and then invokes
save_cwd to save the working directory.  But save_cwd consumes a file
descriptor, so it fails with errno == EMFILE.  The fix is to invoke
save_cwd before invoking 'dup'.

Could you please try the following patch, which implements this fix?
I am enclosing a copy of the new fdopendir.c; perhaps it'd be easiest
if you simply copied this new version over tar-1.25/gnu/fdopendir.c,
and then rebuilt 'tar'.

Thanks.

From 83fb034e8c087a7278c0d3a5686d9c384afa34b8 Mon Sep 17 00:00:00 2001
From: Paul Eggert <eggert@cs.ucla.edu>
Date: Sun, 7 Nov 2010 21:58:54 -0800
Subject: [PATCH] fdopendir: fix bug on MacOS X when low on file descriptors

* lib/fdopendir.c (REPLACE_FCHDIR): #define to 0 if not defined.
(fdopendir_with_dup, fd_clone_opendir): Now have extra CWD arg.
All callers changed.
(fdopendir): Invoke save_cwd at the top level, not after using
multiple dup() calls to use up file descriptors.  Then retry
fdopendir_with_dup.  This avoids failure with EMFILE if FD is 1
less than the maximum number of open file descriptors, because
save_cwd fails with errno == EMFILE.  Problem reported by tsteven4
on Mac OS X 10.6.4 for tar 1.24
<http://lists.gnu.org/archive/html/bug-tar/2010-10/msg00084.html>
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00000.html>
and for tar 1.25
<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00038.html>.
---
 ChangeLog       |   18 ++++++++
 lib/fdopendir.c |  124 ++++++++++++++++++++++++++----------------------------
 2 files changed, 78 insertions(+), 64 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4eb90a3..4d9c4ea 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,21 @@
+2010-11-07  Paul Eggert  <eggert@cs.ucla.edu>
+
+	fdopendir: fix bug on MacOS X when low on file descriptors
+
+	* lib/fdopendir.c (REPLACE_FCHDIR): #define to 0 if not defined.
+	(fdopendir_with_dup, fd_clone_opendir): Now have extra CWD arg.
+	All callers changed.
+	(fdopendir): Invoke save_cwd at the top level, not after using
+	multiple dup() calls to use up file descriptors.  Then retry
+	fdopendir_with_dup.  This avoids failure with EMFILE if FD is 1
+	less than the maximum number of open file descriptors, because
+	save_cwd fails with errno == EMFILE.  Problem reported by tsteven4
+	on Mac OS X 10.6.4 for tar 1.24
+	<http://lists.gnu.org/archive/html/bug-tar/2010-10/msg00084.html>
+	<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00000.html>
+	and for tar 1.25
+	<http://lists.gnu.org/archive/html/bug-tar/2010-11/msg00038.html>.
+
 2010-11-07  Bruno Haible  <bruno@clisp.org>
 
 	vasnprintf: Support I flag on glibc systems.
diff --git a/lib/fdopendir.c b/lib/fdopendir.c
index 4d2935f..e565087 100644
--- a/lib/fdopendir.c
+++ b/lib/fdopendir.c
@@ -33,12 +33,16 @@
 #  include "dirent--.h"
 # endif
 
-static DIR *fdopendir_with_dup (int, int);
-static DIR *fd_clone_opendir (int);
+# ifndef REPLACE_FCHDIR
+#  define REPLACE_FCHDIR 0
+# endif
+
+static DIR *fdopendir_with_dup (int, int, struct saved_cwd const *);
+static DIR *fd_clone_opendir (int, struct saved_cwd const *);
 
 /* Replacement for POSIX fdopendir.
 
-   First, try to simulate it via opendir ("/proc/self/fd/FD").  Failing
+   First, try to simulate it via opendir ("/proc/self/fd/...").  Failing
    that, simulate it by using fchdir metadata, or by doing
    save_cwd/fchdir/opendir(".")/restore_cwd.
    If either the save_cwd or the restore_cwd fails (relatively unlikely),
@@ -61,7 +65,24 @@ static DIR *fd_clone_opendir (int);
 DIR *
 fdopendir (int fd)
 {
-  return fdopendir_with_dup (fd, -1);
+  DIR *dir = fdopendir_with_dup (fd, -1, NULL);
+
+  if (! REPLACE_FCHDIR && ! dir)
+    {
+      int saved_errno = errno;
+      if (EXPECTED_ERRNO (saved_errno))
+        {
+          struct saved_cwd cwd;
+          if (save_cwd (&cwd) != 0)
+            openat_save_fail (errno);
+          dir = fdopendir_with_dup (fd, -1, &cwd);
+          saved_errno = errno;
+          free_cwd (&cwd);
+          errno = saved_errno;
+        }
+    }
+
+  return dir;
 }
 
 /* Like fdopendir, except that if OLDER_DUPFD is not -1, it is known
@@ -70,9 +91,13 @@ fdopendir (int fd)
    function makes sure that FD is closed and all file descriptors less
    than FD are open, and then calls fd_clone_opendir on a dup of FD.
    That way, barring race conditions, fd_clone_opendir returns a
-   stream whose file descriptor is FD.  */
+   stream whose file descriptor is FD.
+
+   If REPLACE_CHDIR or CWD is null, use opendir ("/proc/self/fd/...",
+   falling back on fchdir metadata.  Otherwise, CWD is a saved version
+   of the working directory; use fchdir/opendir(".")/restore_cwd(CWD).  */
 static DIR *
-fdopendir_with_dup (int fd, int older_dupfd)
+fdopendir_with_dup (int fd, int older_dupfd, struct saved_cwd const *cwd)
 {
   int dupfd = dup (fd);
   if (dupfd < 0 && errno == EMFILE)
@@ -85,13 +110,13 @@ fdopendir_with_dup (int fd, int older_dupfd)
       int saved_errno;
       if (dupfd < fd - 1 && dupfd != older_dupfd)
         {
-          dir = fdopendir_with_dup (fd, dupfd);
+          dir = fdopendir_with_dup (fd, dupfd, cwd);
           saved_errno = errno;
         }
       else
         {
           close (fd);
-          dir = fd_clone_opendir (dupfd);
+          dir = fd_clone_opendir (dupfd, cwd);
           saved_errno = errno;
           if (! dir)
             {
@@ -112,74 +137,45 @@ fdopendir_with_dup (int fd, int older_dupfd)
    the caller's responsibility both to close FD and (if the result is
    not null) to closedir the result.  */
 static DIR *
-fd_clone_opendir (int fd)
+fd_clone_opendir (int fd, struct saved_cwd const *cwd)
 {
-  int saved_errno;
-  DIR *dir;
-
-  char buf[OPENAT_BUFFER_SIZE];
-  char *proc_file = openat_proc_name (buf, fd, ".");
-  if (proc_file)
-    {
-      dir = opendir (proc_file);
-      saved_errno = errno;
-    }
-  else
+  if (REPLACE_FCHDIR || ! cwd)
     {
-      dir = NULL;
-      saved_errno = EOPNOTSUPP;
-    }
-
-  /* If the syscall fails with an expected errno value, resort to
-     save_cwd/restore_cwd.  */
-  if (! dir && EXPECTED_ERRNO (saved_errno))
-    {
-# if REPLACE_FCHDIR
-      const char *name = _gl_directory_name (fd);
-      if (name)
-        dir = opendir (name);
-      saved_errno = errno;
-# else /* !REPLACE_FCHDIR */
-
-      /* Occupy the destination FD slot, so that save_cwd cannot hijack it.  */
-      struct saved_cwd saved_cwd;
-      int fd_reserve = dup (fd);
-      if (fd_reserve < 0)
+      DIR *dir = NULL;
+      int saved_errno = EOPNOTSUPP;
+      char buf[OPENAT_BUFFER_SIZE];
+      char *proc_file = openat_proc_name (buf, fd, ".");
+      if (proc_file)
         {
+          dir = opendir (proc_file);
           saved_errno = errno;
-          dir = NULL;
-          goto fail;
+          if (proc_file != buf)
+            free (proc_file);
         }
-
-      if (save_cwd (&saved_cwd) != 0)
-        openat_save_fail (errno);
-
-      /* Liberate the target file descriptor, so that opendir uses it.  */
-      close (fd_reserve);
-
-      if (fchdir (fd) != 0)
+# if REPLACE_FCHDIR
+      if (! dir && EXPECTED_ERRNO (saved_errno))
         {
-          dir = NULL;
-          saved_errno = errno;
+          char const *name = _gl_directory_name (fd);
+          return (name ? opendir (name) : NULL);
         }
+# endif
+      errno = saved_errno;
+      return dir;
+    }
+  else
+    {
+      if (fchdir (fd) != 0)
+        return NULL;
       else
         {
-          dir = opendir (".");
-          saved_errno = errno;
-
-          if (restore_cwd (&saved_cwd) != 0)
+          DIR *dir = opendir (".");
+          int saved_errno = errno;
+          if (restore_cwd (cwd) != 0)
             openat_restore_fail (errno);
+          errno = saved_errno;
+          return dir;
         }
-
-      free_cwd (&saved_cwd);
-# endif /* !REPLACE_FCHDIR */
     }
-
- fail:
-  if (proc_file != buf)
-    free (proc_file);
-  errno = saved_errno;
-  return dir;
 }
 
 #else /* HAVE_FDOPENDIR */
-- 
1.7.2



["fdopendir.c" (text/x-csrc)]

/* provide a replacement fdopendir function
   Copyright (C) 2004-2010 Free Software Foundation, Inc.

   This program is free software: you can redistribute it and/or modify
   it under the terms of the GNU General Public License as published by
   the Free Software Foundation; either version 3 of the License, or
   (at your option) any later version.

   This program is distributed in the hope that it will be useful,
   but WITHOUT ANY WARRANTY; without even the implied warranty of
   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
   GNU General Public License for more details.

   You should have received a copy of the GNU General Public License
   along with this program.  If not, see <http://www.gnu.org/licenses/>.  */

/* written by Jim Meyering */

#include <config.h>

#include <dirent.h>

#include <stdlib.h>
#include <unistd.h>

#if !HAVE_FDOPENDIR

# include "openat.h"
# include "openat-priv.h"
# include "save-cwd.h"

# if GNULIB_DIRENT_SAFER
#  include "dirent--.h"
# endif

# ifndef REPLACE_FCHDIR
#  define REPLACE_FCHDIR 0
# endif

static DIR *fdopendir_with_dup (int, int, struct saved_cwd const *);
static DIR *fd_clone_opendir (int, struct saved_cwd const *);

/* Replacement for POSIX fdopendir.

   First, try to simulate it via opendir ("/proc/self/fd/...").  Failing
   that, simulate it by using fchdir metadata, or by doing
   save_cwd/fchdir/opendir(".")/restore_cwd.
   If either the save_cwd or the restore_cwd fails (relatively unlikely),
   then give a diagnostic and exit nonzero.

   If successful, the resulting stream is based on FD in
   implementations where streams are based on file descriptors and in
   applications where no other thread or signal handler allocates or
   frees file descriptors.  In other cases, consult dirfd on the result
   to find out whether FD is still being used.

   Otherwise, this function works just like POSIX fdopendir.

   W A R N I N G:

   Unlike other fd-related functions, this one places constraints on FD.
   If this function returns successfully, FD is under control of the
   dirent.h system, and the caller should not close or modify the state of
   FD other than by the dirent.h functions.  */
DIR *
fdopendir (int fd)
{
  DIR *dir = fdopendir_with_dup (fd, -1, NULL);

  if (! REPLACE_FCHDIR && ! dir)
    {
      int saved_errno = errno;
      if (EXPECTED_ERRNO (saved_errno))
        {
          struct saved_cwd cwd;
          if (save_cwd (&cwd) != 0)
            openat_save_fail (errno);
          dir = fdopendir_with_dup (fd, -1, &cwd);
          saved_errno = errno;
          free_cwd (&cwd);
          errno = saved_errno;
        }
    }

  return dir;
}

/* Like fdopendir, except that if OLDER_DUPFD is not -1, it is known
   to be a dup of FD which is less than FD - 1 and which will be
   closed by the caller and not otherwise used by the caller.  This
   function makes sure that FD is closed and all file descriptors less
   than FD are open, and then calls fd_clone_opendir on a dup of FD.
   That way, barring race conditions, fd_clone_opendir returns a
   stream whose file descriptor is FD.

   If REPLACE_CHDIR or CWD is null, use opendir ("/proc/self/fd/...",
   falling back on fchdir metadata.  Otherwise, CWD is a saved version
   of the working directory; use fchdir/opendir(".")/restore_cwd(CWD).  */
static DIR *
fdopendir_with_dup (int fd, int older_dupfd, struct saved_cwd const *cwd)
{
  int dupfd = dup (fd);
  if (dupfd < 0 && errno == EMFILE)
    dupfd = older_dupfd;
  if (dupfd < 0)
    return NULL;
  else
    {
      DIR *dir;
      int saved_errno;
      if (dupfd < fd - 1 && dupfd != older_dupfd)
        {
          dir = fdopendir_with_dup (fd, dupfd, cwd);
          saved_errno = errno;
        }
      else
        {
          close (fd);
          dir = fd_clone_opendir (dupfd, cwd);
          saved_errno = errno;
          if (! dir)
            {
              int fd1 = dup (dupfd);
              if (fd1 != fd)
                openat_save_fail (fd1 < 0 ? errno : EBADF);
            }
        }

      if (dupfd != older_dupfd)
        close (dupfd);
      errno = saved_errno;
      return dir;
    }
}

/* Like fdopendir, except the result controls a clone of FD.  It is
   the caller's responsibility both to close FD and (if the result is
   not null) to closedir the result.  */
static DIR *
fd_clone_opendir (int fd, struct saved_cwd const *cwd)
{
  if (REPLACE_FCHDIR || ! cwd)
    {
      DIR *dir = NULL;
      int saved_errno = EOPNOTSUPP;
      char buf[OPENAT_BUFFER_SIZE];
      char *proc_file = openat_proc_name (buf, fd, ".");
      if (proc_file)
        {
          dir = opendir (proc_file);
          saved_errno = errno;
          if (proc_file != buf)
            free (proc_file);
        }
# if REPLACE_FCHDIR
      if (! dir && EXPECTED_ERRNO (saved_errno))
        {
          char const *name = _gl_directory_name (fd);
          return (name ? opendir (name) : NULL);
        }
# endif
      errno = saved_errno;
      return dir;
    }
  else
    {
      if (fchdir (fd) != 0)
        return NULL;
      else
        {
          DIR *dir = opendir (".");
          int saved_errno = errno;
          if (restore_cwd (cwd) != 0)
            openat_restore_fail (errno);
          errno = saved_errno;
          return dir;
        }
    }
}

#else /* HAVE_FDOPENDIR */

# include <errno.h>
# include <sys/stat.h>

# undef fdopendir

/* Like fdopendir, but work around GNU/Hurd bug by validating FD.  */

DIR *
rpl_fdopendir (int fd)
{
  struct stat st;
  if (fstat (fd, &st))
    return NULL;
  if (!S_ISDIR (st.st_mode))
    {
      errno = ENOTDIR;
      return NULL;
    }
  return fdopendir (fd);
}

#endif /* HAVE_FDOPENDIR */


[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic