fdopendir() wrongly closes passed fd on error, union mess

[Jilles Tjoelker]

While trying to use openat()/fdopendir()/fstatat() to improve
performance of sh(1) pathname generation, I noticed that fdopendir()
closes the passed file descriptor if it fails (such as when stable/8
silently ignores O_DIRECTORY even though it is defined in the header
file).

The below patch should fix this problem. I have changed the ugly
DTF_REWIND code so it moves the reopened directory to the same file
descriptor number, so as to minimize change there while ensuring the
correct fd is closed.

A later possibility is to fix the DTF_REWIND problem by reading union
directories from a new descriptor obtained via
  fd2 = openat(fd, ".", O_RDONLY | O_DIRECTORY | O_CLOEXEC);
and doing this unconditionally, retiring the DTF_REWIND flag.

Reading the kernel code, it appears that calling getdirentries() on an
open file description in a filesystem mounted with MNT_UNION (mount -o
union) may change that open file description irreversibly to one
pointing to the lower layer. This must not happen to the descriptor
passed to fdopendir() or the descriptor returned via dirfd() since
fchdir() and *at functions may not work properly with such a changed
descriptor.

Note: unionfs does not appear to mangle the open file description like
MNT_UNION does, but it does need the duplicate removal code like
MNT_UNION.

Index: lib/libc/gen/opendir.c
===================================================================
--- lib/libc/gen/opendir.c	(revision 224739)
+++ lib/libc/gen/opendir.c	(working copy)
@@ -75,6 +75,8 @@ __opendir2(const char *name, int flags)
 {
 	int fd;
 	struct stat statb;
+	DIR *dir;
+	int saved_errno;
 
 	/*
 	 * stat() before _open() because opening of special files may be
@@ -89,7 +91,13 @@ __opendir2(const char *name, int flags)
 	if ((fd = _open(name, O_RDONLY | O_NONBLOCK | O_DIRECTORY)) == -1)
 		return (NULL);
 
-	return __opendir_common(fd, name, flags);
+	dir = __opendir_common(fd, name, flags);
+	if (dir == NULL) {
+		saved_errno = errno;
+		_close(fd);
+		errno = saved_errno;
+	}
+	return (dir);
 }
 
 static int
@@ -110,6 +118,7 @@ __opendir_common(int fd, const char *name, int fla
 	int incr;
 	int saved_errno;
 	int unionstack;
+	int fd2;
 	struct stat statb;
 
 	dirp = NULL;
@@ -199,14 +208,15 @@ __opendir_common(int fd, const char *name, int fla
 		 * which has also been read -- see fts.c.
 		 */
 		if (flags & DTF_REWIND) {
-			(void)_close(fd);
-			if ((fd = _open(name, O_RDONLY | O_DIRECTORY)) == -1) {
+			if ((fd2 = _open(name, O_RDONLY | O_DIRECTORY)) == -1) {
 				saved_errno = errno;
 				free(buf);
 				free(dirp);
 				errno = saved_errno;
 				return (NULL);
 			}
+			(void)_dup2(fd2, fd);
+			_close(fd2);
 		}
 
 		/*
@@ -309,7 +319,6 @@ __opendir_common(int fd, const char *name, int fla
 fail:
 	saved_errno = errno;
 	free(dirp);
-	(void)_close(fd);
 	errno = saved_errno;
 	return (NULL);
 }

-- 
Jilles Tjoelker