getdirentries: Return ENOTDIR if not a directory.

This is both more logical and more useful than EINVAL.

While here, also check for VBAD and return EBADF in that case.  This can
happen if the underlying filesystem got forcibly unmounted after the
directory was opened.  Previously, this would also have returned EINVAL,
which wasn't right but wasn't wrong either; however, ENOTDIR would not
be appropriate.

MFC after:	never
Sponsored by:	Klara, Inc.
Reviewed by:	kevans, kib
Differential Revision:	https://reviews.freebsd.org/D51209

lib/libc/gen/opendir2.c

@@ -315,11 +315,8 @@ __opendir_common(int fd, int flags, bool use_current_pos)
 			 */
 			dirp->dd_size = _getdirentries(dirp->dd_fd,
 			    dirp->dd_buf, dirp->dd_len, &dirp->dd_seek);
-			if (dirp->dd_size < 0) {
-				if (errno == EINVAL)
-					errno = ENOTDIR;
+			if (dirp->dd_size < 0)
 				goto fail;
-			}
 			dirp->dd_flags |= __DTF_SKIPREAD;
 		} else {
 			dirp->dd_size = 0;

lib/libsys/getdirentries.2

@@ -25,7 +25,7 @@
 .\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
 .\" SUCH DAMAGE.
 .\"
-.Dd September 5, 2023
+.Dd July 8, 2025
 .Dt GETDIRENTRIES 2
 .Os
 .Sh NAME
@@ -178,9 +178,7 @@ or non-NULL
 .Fa basep
 point outside the allocated address space.
 .It Bq Er EINVAL
-The file referenced by
-.Fa fd
-is not a directory, or
+The value of
 .Fa nbytes
 is too small for returning a directory entry or block of entries,
 or the current position pointer is invalid.
@@ -192,6 +190,10 @@ error occurred while reading from or writing to the file system.
 Corrupted data was detected while reading from the file system.
 .It Bq Er ENOENT
 Directory unlinked but still open.
+.It Bq Er ENOTDIR
+The file referenced by
+.Fa fd
+is not a directory.
 .El
 .Sh SEE ALSO
 .Xr lseek 2 ,

sys/kern/vfs_syscalls.c

@@ -4314,10 +4314,6 @@ kern_getdirentries(struct thread *td, int fd, char *buf, size_t count,
 	vp = fp->f_vnode;
 	foffset = foffset_lock(fp, 0);
 unionread:
-	if (vp->v_type != VDIR) {
-		error = EINVAL;
-		goto fail;
-	}
 	if (__predict_false((vp->v_vflag & VV_UNLINKED) != 0)) {
 		error = ENOENT;
 		goto fail;
@@ -4330,6 +4326,19 @@ unionread:
 	auio.uio_segflg = bufseg;
 	auio.uio_td = td;
 	vn_lock(vp, LK_SHARED | LK_RETRY);
+	/*
+	 * We want to return ENOTDIR for anything that is not VDIR, but
+	 * not for VBAD, and we can't check for VBAD while the vnode is
+	 * unlocked.
+	 */
+	if (vp->v_type != VDIR) {
+		if (vp->v_type == VBAD)
+			error = EBADF;
+		else
+			error = ENOTDIR;
+		VOP_UNLOCK(vp);
+		goto fail;
+	}
 	AUDIT_ARG_VNODE1(vp);
 	loff = auio.uio_offset = foffset;
 #ifdef MAC

tests/sys/kern/Makefile

@@ -18,6 +18,7 @@ ATF_TESTS_C+=	kern_descrip_test
 # One test modifies the maxfiles limit, which can cause spurious test failures.
 TEST_METADATA.kern_descrip_test+= is_exclusive="true"
 ATF_TESTS_C+=	fdgrowtable_test
+ATF_TESTS_C+=	getdirentries_test
 ATF_TESTS_C+=	jail_lookup_root
 ATF_TESTS_C+=	inotify_test
 ATF_TESTS_C+=	kill_zombie

tests/sys/kern/getdirentries_test.c

@@ -0,0 +1,172 @@
+/*-
+ * Copyright (c) 2025 Klara, Inc.
+ *
+ * SPDX-License-Identifier: BSD-2-Clause
+ */
+
+#include <sys/stat.h>
+#include <sys/mount.h>
+
+#include <dirent.h>
+#include <fcntl.h>
+#include <errno.h>
+#include <stdint.h>
+
+#include <atf-c.h>
+
+ATF_TC(getdirentries_ok);
+ATF_TC_HEAD(getdirentries_ok, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Successfully read a directory.");
+}
+ATF_TC_BODY(getdirentries_ok, tc)
+{
+	char dbuf[4096];
+	struct dirent *d;
+	off_t base;
+	ssize_t ret;
+	int dd, n;
+
+	ATF_REQUIRE_EQ(0, mkdir("dir", 0755));
+	ATF_REQUIRE((dd = open("dir", O_DIRECTORY | O_RDONLY)) >= 0);
+	ATF_REQUIRE((ret = getdirentries(dd, dbuf, sizeof(dbuf), &base)) > 0);
+	ATF_REQUIRE_EQ(0, getdirentries(dd, dbuf, sizeof(dbuf), &base));
+	ATF_REQUIRE_EQ(base, lseek(dd, 0, SEEK_CUR));
+	ATF_CHECK_EQ(0, close(dd));
+	for (n = 0, d = (struct dirent *)dbuf;
+	     d < (struct dirent *)(dbuf + ret);
+	     d = (struct dirent *)((char *)d + d->d_reclen), n++)
+		/* nothing */ ;
+	ATF_CHECK_EQ((struct dirent *)(dbuf + ret), d);
+	ATF_CHECK_EQ(2, n);
+}
+
+ATF_TC(getdirentries_ebadf);
+ATF_TC_HEAD(getdirentries_ebadf, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Attempt to read a directory "
+	    "from an invalid descriptor.");
+}
+ATF_TC_BODY(getdirentries_ebadf, tc)
+{
+	char dbuf[4096];
+	off_t base;
+	int fd;
+
+	ATF_REQUIRE((fd = open("file", O_CREAT | O_WRONLY, 0644)) >= 0);
+	ATF_REQUIRE_EQ(-1, getdirentries(fd, dbuf, sizeof(dbuf), &base));
+	ATF_CHECK_EQ(EBADF, errno);
+	ATF_REQUIRE_EQ(0, close(fd));
+	ATF_REQUIRE_EQ(-1, getdirentries(fd, dbuf, sizeof(dbuf), &base));
+	ATF_CHECK_EQ(EBADF, errno);
+}
+
+ATF_TC(getdirentries_efault);
+ATF_TC_HEAD(getdirentries_efault, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Attempt to read a directory "
+	    "to an invalid buffer.");
+}
+ATF_TC_BODY(getdirentries_efault, tc)
+{
+	char dbuf[4096];
+	off_t base, *basep;
+	int dd;
+
+	ATF_REQUIRE_EQ(0, mkdir("dir", 0755));
+	ATF_REQUIRE((dd = open("dir", O_DIRECTORY | O_RDONLY)) >= 0);
+	ATF_REQUIRE_EQ(-1, getdirentries(dd, NULL, sizeof(dbuf), &base));
+	ATF_CHECK_EQ(EFAULT, errno);
+	basep = NULL;
+	basep++;
+	ATF_REQUIRE_EQ(-1, getdirentries(dd, dbuf, sizeof(dbuf), basep));
+	ATF_CHECK_EQ(EFAULT, errno);
+	ATF_CHECK_EQ(0, close(dd));
+}
+
+ATF_TC(getdirentries_einval);
+ATF_TC_HEAD(getdirentries_einval, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Attempt to read a directory "
+	    "with various invalid parameters.");
+}
+ATF_TC_BODY(getdirentries_einval, tc)
+{
+	struct statfs fsb;
+	char dbuf[4096];
+	off_t base;
+	ssize_t ret;
+	int dd;
+
+	ATF_REQUIRE_EQ(0, mkdir("dir", 0755));
+	ATF_REQUIRE((dd = open("dir", O_DIRECTORY | O_RDONLY)) >= 0);
+	ATF_REQUIRE_EQ(0, fstatfs(dd, &fsb));
+	/* nbytes too small */
+	ATF_REQUIRE_EQ(-1, getdirentries(dd, dbuf, 8, &base));
+	ATF_CHECK_EQ(EINVAL, errno);
+	/* nbytes too big */
+	ATF_REQUIRE_EQ(-1, getdirentries(dd, dbuf, SIZE_MAX, &base));
+	ATF_CHECK_EQ(EINVAL, errno);
+	/* invalid position */
+	ATF_REQUIRE((ret = getdirentries(dd, dbuf, sizeof(dbuf), &base)) > 0);
+	ATF_REQUIRE_EQ(0, getdirentries(dd, dbuf, sizeof(dbuf), &base));
+	ATF_REQUIRE(base > 0);
+	ATF_REQUIRE_EQ(base + 3, lseek(dd, 3, SEEK_CUR));
+	/* known to fail on ufs (FFS2) and zfs, and work on tmpfs */
+	if (strcmp(fsb.f_fstypename, "ufs") == 0 ||
+	    strcmp(fsb.f_fstypename, "zfs") == 0) {
+		atf_tc_expect_fail("incorrectly returns 0 instead of EINVAL "
+		    "on %s", fsb.f_fstypename);
+	}
+	ATF_REQUIRE_EQ(-1, getdirentries(dd, dbuf, sizeof(dbuf), &base));
+	ATF_CHECK_EQ(EINVAL, errno);
+	ATF_CHECK_EQ(0, close(dd));
+}
+
+ATF_TC(getdirentries_enoent);
+ATF_TC_HEAD(getdirentries_enoent, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Attempt to read a directory "
+	    "after it is deleted.");
+}
+ATF_TC_BODY(getdirentries_enoent, tc)
+{
+	char dbuf[4096];
+	off_t base;
+	int dd;
+
+	ATF_REQUIRE_EQ(0, mkdir("dir", 0755));
+	ATF_REQUIRE((dd = open("dir", O_DIRECTORY | O_RDONLY)) >= 0);
+	ATF_REQUIRE_EQ(0, rmdir("dir"));
+	ATF_REQUIRE_EQ(-1, getdirentries(dd, dbuf, sizeof(dbuf), &base));
+	ATF_CHECK_EQ(ENOENT, errno);
+}
+
+ATF_TC(getdirentries_enotdir);
+ATF_TC_HEAD(getdirentries_enotdir, tc)
+{
+	atf_tc_set_md_var(tc, "descr", "Attempt to read a directory "
+	    "from a descriptor not associated with a directory.");
+}
+ATF_TC_BODY(getdirentries_enotdir, tc)
+{
+	char dbuf[4096];
+	off_t base;
+	int fd;
+
+	ATF_REQUIRE((fd = open("file", O_CREAT | O_RDWR, 0644)) >= 0);
+	ATF_REQUIRE_EQ(-1, getdirentries(fd, dbuf, sizeof(dbuf), &base));
+	ATF_CHECK_EQ(ENOTDIR, errno);
+	ATF_CHECK_EQ(0, close(fd));
+}
+
+ATF_TP_ADD_TCS(tp)
+{
+	ATF_TP_ADD_TC(tp, getdirentries_ok);
+	ATF_TP_ADD_TC(tp, getdirentries_ebadf);
+	ATF_TP_ADD_TC(tp, getdirentries_efault);
+	ATF_TP_ADD_TC(tp, getdirentries_einval);
+	ATF_TP_ADD_TC(tp, getdirentries_enoent);
+	ATF_TP_ADD_TC(tp, getdirentries_enotdir);
+	return (atf_no_error());
+}