Re: several changes today

[Zack Weinberg <zack@rabi.columbia.edu>]

On Sun, 18 Oct 1998 22:08:24 -0400 (EDT), Thomas Bushnell, BSG wrote:
>   Date: Sun, 18 Oct 1998 20:04:58 -0400
>   From: Roland McGrath <roland@frob.com>
>
>   Of course I checked the standard before making authoritative claims about
>   its contents, silly.  See 1003.1-1996 5.1.2.4, p 116.  The only "if
>   detected" errors listed for `opendir' are EMFILE and ENFILE.  Listed
>   as "if occur" errors are EACCES, ENAMETOOLONG, ENOENT, and ENOTDIR.
>
>I just didn't have the standard at hand at it wasn't clear to me from
>your messages whether you were making an authoritative claim or not...
>
>So I agree, for the record, with your comments in this thread; on a
>generic system (lacking O_DIRECTORY or other special features) an
>fstat after the open is 100% obligatory.

We really need to test for O_DIRECTORY at runtime.  Older versions of
Linux provide equivalent functionality by appending a slash to the
filename, so we should look for that too.  If neither mechanism is
available, we ought to stat before and after.  Here is a rewrite which
does all these and fixes some bugs too (not preserving errno
correctly, variables not necessarily initialized).  A potential issue
is that /dev/null isn't guaranteed to exist, but realistically this is
only a problem in chroot jails, and I've arranged to fail safe.

The patch is not tested yet - I'm recompiling now.

zw

1998-10-18 23:25 -0400  Zack Weinberg  <zack@rabi.phys.columbia.edu>

	* sysdeps/unix/opendir.c: Check at runtime for kernel support for
	O_DIRECTORY or the appended-slash hack.  Make sure not to open
	anything other than a directory, by stat()ing before and after
	the open if necessary.  Clean up.

============================================================
Index: sysdeps/unix/opendir.c
--- sysdeps/unix/opendir.c	1998/10/16 16:00:50	1.23
+++ sysdeps/unix/opendir.c	1998/10/19 03:20:08
@@ -16,52 +16,146 @@
    write to the Free Software Foundation, Inc., 59 Temple Place - Suite 330,
    Boston, MA 02111-1307, USA.  */
 
+#include <stddef.h>
+#include <sys/types.h>
+#include <sys/stat.h>
 #include <errno.h>
 #include <limits.h>
-#include <stddef.h>
 #include <stdlib.h>
+#include <string.h>
 #include <dirent.h>
 #include <fcntl.h>
-#include <sys/types.h>
-#include <sys/stat.h>
 #include <unistd.h>
 #include <stdio.h>
 
 #include <dirstream.h>
-
 
-/* We want to be really safe the file we opened is a directory.  Some systems
-   have support for this, others don't.  */
+/* opendir() must not accidentally open something other than a directory.
+   Some OS's have kernel support for that, some don't.  In the worst
+   case we have to stat() before the open() AND fstat() after.
+
+   We have to test at runtime for kernel support since libc may have
+   been compiled with different headers to the kernel it's running on.
+   This test can't be done reliably in the general case.  We'll use
+   /dev/null, which if it's not a device lots of stuff will break, as
+   a guinea pig.  It may be missing in chroot environments, so we
+   make sure to fail safe. */
 #ifdef O_DIRECTORY
-# define OPENDIR(NAME) \
-  do {									      \
-    fd = __open (NAME, O_RDONLY|O_NDELAY|O_DIRECTORY);			      \
-    if (fd < 0)								      \
-      return NULL;							      \
-  } while (0)
-#else
-# define OPENDIR(NAME) \
-  do {									      \
-    fd = __open (NAME, O_RDONLY|O_NDELAY);				      \
-    if (fd < 0 || __fstat (fd, &statbuf) < 0 || ! S_ISDIR (statbuf.st_mode))  \
-      {									      \
-	if (fd >= 0)							      \
-	  __close (fd);							      \
-	return NULL;							      \
-      }									      \
-  } while (0)
+static int
+tryopen_o_directory (const char *name)
+{
+  static int o_directory_works = -1;
+
+  if (o_directory_works == -1)
+    {
+      int serrno = errno;
+      int x = __open ("/dev/null", O_RDONLY|O_NDELAY|O_DIRECTORY);
+
+      if (x >= 0)
+        {
+	  __close (x);
+	  o_directory_works = 0;
+	}
+      else if (errno != ENOTDIR)
+	o_directory_works = 0;
+      else
+	o_directory_works = 1;
+
+      __set_errno (serrno);
+    }
+
+  if (o_directory_works)
+    return __open (name, O_RDONLY|O_NDELAY|O_DIRECTORY);
+  else
+    return -2;
+}
 #endif
 
+static int
+tryopen_append_slash (const char *name)
+{
+  static int append_slash_works = -1;
+
+  if (append_slash_works == -1)
+    {
+      int serrno = errno;
+      int x = __open ("/dev/null/", O_RDONLY|O_NDELAY);
+
+      if (x >= 0)
+        {
+	  __close (x);
+	  append_slash_works = 0;
+	}
+      else if (errno != ENOTDIR)
+	append_slash_works = 0;
+      else
+	append_slash_works = 1;
+
+      __set_errno (serrno);
+    }
+
+  if (append_slash_works)
+    {
+      size_t namelen = strlen (name);
+      char *namex = alloca (namelen + 2);
+
+      strcpy (namex, name);
+      namex[namelen-2] = '/';
+      namex[namelen-1] = '\0';
+
+      return __open (namex, O_RDONLY|O_NDELAY);
+    }
+  else
+    return -2;
+}
+
+static int
+tryopen_check_by_hand (const char *name)
+{
+  struct stat st;
+  int fd;
+  int serrno = errno;
+  /* We first have to check whether the name is for a directory.  We
+     cannot do this after the open() call since the open/close operation
+     performed on, say, a tape device might have undesirable effects.  */
+  if (stat (name, &st) < 0)
+    return -1;
+  if (! S_ISDIR (st.st_mode))
+    {
+      __set_errno (ENOTDIR);
+      return -1;
+    }
+
+  fd = __open (name, O_RDONLY|O_NDELAY);
 
+  /* We have to check again after the open, because the target might've
+     changed between the stat and the open. */
+  if (fd < 0)
+    return -1;
+  if (fstat (fd, &st) < 0)
+    {
+      __close (fd);
+      return -1;
+    }
+  if (! S_ISDIR (st.st_mode))
+    {
+      __close (fd);
+      __set_errno (ENOTDIR);
+      return -1;
+    }
+
+  __set_errno (serrno);
+  return fd;
+}
+
+
 /* Open a directory stream on NAME.  */
 DIR *
 __opendir (const char *name)
 {
   DIR *dirp;
-  struct stat statbuf;
   int fd;
   size_t allocation;
-  int save_errno;
 
   if (name[0] == '\0')
     {
@@ -71,41 +165,33 @@
       return NULL;
     }
 
-  /* We first have to check whether the name is for a directory.  We
-     cannot do this after the open() call since the open/close operation
-     performed on, say, a tape device might have undesirable effects.  */
-  if (stat (name, &statbuf) < 0)
+  allocation = (BUFSIZ < sizeof (struct dirent)
+		? sizeof (struct dirent) : BUFSIZ);
+
+  dirp = (DIR *) calloc (1, sizeof (DIR) + allocation);
+  if (dirp == NULL)
     return NULL;
-  if (! S_ISDIR (statbuf.st_mode))
+  
+  fd = -2;
+#ifdef O_DIRECTORY
+  fd = tryopen_o_directory (name);
+#endif
+  if (fd == -2) fd = tryopen_append_slash (name);
+  if (fd == -2) fd = tryopen_check_by_hand (name);
+
+  if (fd < 0)
     {
-      __set_errno (ENOTDIR);
+      free (dirp);
       return NULL;
     }
-
-  OPENDIR (name);
-
+  
   if (__fcntl (fd, F_SETFD, FD_CLOEXEC) < 0)
-    goto lose;
-
-#ifdef _STATBUF_ST_BLKSIZE
-  if (statbuf.st_blksize < sizeof (struct dirent))
-    allocation = sizeof (struct dirent);
-  else
-    allocation = statbuf.st_blksize;
-#else
-  allocation = (BUFSIZ < sizeof (struct dirent)
-		? sizeof (struct dirent) : BUFSIZ);
-#endif
-
-  dirp = (DIR *) calloc (1, sizeof (DIR) + allocation); /* Zero-fill.  */
-  if (dirp == NULL)
-  lose:
     {
-      save_errno = errno;
-      (void) __close (fd);
-      __set_errno (save_errno);
+      __close (fd);
+      free (dirp);
       return NULL;
     }
+
   dirp->data = (char *) (dirp + 1);
   dirp->allocation = allocation;
   dirp->fd = fd;