git: 62e0f12f5104 - main - scandir: Propagate errors from readdir().
- Go to: [ bottom of page ] [ top of archives ] [ this month ]
Date: Thu, 26 Jun 2025 07:38:03 UTC
The branch main has been updated by des:
URL: https://cgit.FreeBSD.org/src/commit/?id=62e0f12f5104585b7346fee183e5c667b39ddbad
commit 62e0f12f5104585b7346fee183e5c667b39ddbad
Author: Dag-Erling Smørgrav <des@FreeBSD.org>
AuthorDate: 2025-06-26 07:37:00 +0000
Commit: Dag-Erling Smørgrav <des@FreeBSD.org>
CommitDate: 2025-06-26 07:37:00 +0000
scandir: Propagate errors from readdir().
Currently, if `readdir()` fails, `scandir()` simply returns a partial
result (or a null result if it fails before any entries were selected).
There is no way within the current API design to return both a partial
result and an error indicator, so err on the side of caution: if an
error occurs, discard any partial result and return the error instead.
MFC after: 1 week
Reported by: Maxim Suhanov <dfirblog@gmail.com>
Sponsored by: Klara, Inc.
Reviewed by: markj
Differential Revision: https://reviews.freebsd.org/D51046
---
lib/libc/gen/scandir.3 | 32 +++++++++++++++++++-
lib/libc/gen/scandir.c | 12 ++++++--
lib/libc/tests/gen/scandir_test.c | 63 +++++++++++++++++++++++++++++++++++++++
3 files changed, 104 insertions(+), 3 deletions(-)
diff --git a/lib/libc/gen/scandir.3 b/lib/libc/gen/scandir.3
index 9ced9fa4ef9d..3da4500cefb9 100644
--- a/lib/libc/gen/scandir.3
+++ b/lib/libc/gen/scandir.3
@@ -25,7 +25,7 @@
.\" OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF
.\" SUCH DAMAGE.
.\"
-.Dd June 20, 2025
+.Dd June 25, 2025
.Dt SCANDIR 3
.Os
.Sh NAME
@@ -215,6 +215,36 @@ cannot allocate enough memory to hold all the directory entries,
they return \-1 and set
.Va errno
to an appropriate value.
+.Sh ERRORS
+The
+.Fn scandir ,
+.Fn scandirat ,
+.Fn scandir_b ,
+and
+.Fn scandirat_b
+functions may fail and set
+.Va errno
+for any of the errors specified for the
+.Xr opendir 3 ,
+.Xr malloc 3 ,
+.Xr readdir 3 ,
+and
+.Xr closedir 3
+functions.
+.Pp
+The
+.Fn fdscandir
+and
+.Fn fdscandir_b
+functions may fail and set
+.Va errno
+for any of the errors specified for the
+.Xr fdopendir 3 ,
+.Xr malloc 3 ,
+.Xr readdir 3 ,
+and
+.Xr closedir 3
+functions.
.Sh SEE ALSO
.Xr openat 2 ,
.Xr directory 3 ,
diff --git a/lib/libc/gen/scandir.c b/lib/libc/gen/scandir.c
index 134c88713d39..56d77c29bd07 100644
--- a/lib/libc/gen/scandir.c
+++ b/lib/libc/gen/scandir.c
@@ -81,7 +81,7 @@ scandir_dirp(DIR *dirp, struct dirent ***namelist,
if (names == NULL)
return (-1);
- while ((d = readdir(dirp)) != NULL) {
+ while (errno = 0, (d = readdir(dirp)) != NULL) {
if (select != NULL && !SELECT(d))
continue; /* just selected names */
/*
@@ -108,6 +108,13 @@ scandir_dirp(DIR *dirp, struct dirent ***namelist,
}
names[numitems++] = p;
}
+ /*
+ * Since we can't simultaneously return both -1 and a count, we
+ * must either suppress the error or discard the partial result.
+ * The latter seems the lesser of two evils.
+ */
+ if (errno != 0)
+ goto fail;
if (numitems && dcomp != NULL)
#ifdef I_AM_SCANDIR_B
qsort_b(names, numitems, sizeof(struct dirent *), (void*)dcomp);
@@ -120,7 +127,8 @@ scandir_dirp(DIR *dirp, struct dirent ***namelist,
fail:
serrno = errno;
- free(p);
+ if (numitems == 0 || names[numitems - 1] != p)
+ free(p);
while (numitems > 0)
free(names[--numitems]);
free(names);
diff --git a/lib/libc/tests/gen/scandir_test.c b/lib/libc/tests/gen/scandir_test.c
index 9a9940aca881..f7b52b5e3616 100644
--- a/lib/libc/tests/gen/scandir_test.c
+++ b/lib/libc/tests/gen/scandir_test.c
@@ -7,7 +7,9 @@
#include <sys/stat.h>
#include <dirent.h>
+#include <errno.h>
#include <fcntl.h>
+#include <stdio.h>
#include <stdlib.h>
#include <atf-c.h>
@@ -124,11 +126,72 @@ ATF_TC_BODY(scandir_none, tc)
free(namelist);
}
+/*
+ * Test that scandir() propagates errors from readdir(): we create a
+ * directory with enough entries that it can't be read in a single
+ * getdirentries() call, then abuse the selection callback to close the
+ * file descriptor scandir() is using after the first call, causing the
+ * next one to fail, and verify that readdir() returns an error instead of
+ * a partial result. We make two passes, one in which nothing was
+ * selected before the error occurred, and one in which everything was.
+ */
+static int scandir_error_count;
+static int scandir_error_fd;
+static int scandir_error_select_return;
+
+static int
+scandir_error_select(const struct dirent *ent __unused)
+{
+ if (scandir_error_count++ == 0)
+ close(scandir_error_fd);
+ return (scandir_error_select_return);
+}
+
+ATF_TC(scandir_error);
+ATF_TC_HEAD(scandir_error, tc)
+{
+ atf_tc_set_md_var(tc, "descr",
+ "Test that scandir() propagates errors from readdir()");
+}
+ATF_TC_BODY(scandir_error, tc)
+{
+ char path[16];
+ struct dirent **namelist = NULL;
+ int fd, i, ret;
+
+ ATF_REQUIRE_EQ(0, mkdir("dir", 0755));
+ for (i = 0; i < 1024; i++) {
+ snprintf(path, sizeof(path), "dir/%04x", i);
+ ATF_REQUIRE_EQ(0, symlink(path + 4, path));
+ }
+
+ /* first pass, select nothing */
+ ATF_REQUIRE((fd = open("dir", O_DIRECTORY | O_RDONLY)) >= 0);
+ scandir_error_count = 0;
+ scandir_error_fd = fd;
+ scandir_error_select_return = 0;
+ ret = fdscandir(fd, &namelist, scandir_error_select, NULL);
+ ATF_CHECK_EQ(-1, ret);
+ ATF_CHECK_ERRNO(EBADF, ret < 0);
+ ATF_CHECK_EQ(NULL, namelist);
+
+ /* second pass, select everything */
+ ATF_REQUIRE((fd = open("dir", O_DIRECTORY | O_RDONLY)) >= 0);
+ scandir_error_count = 0;
+ scandir_error_fd = fd;
+ scandir_error_select_return = 1;
+ ret = fdscandir(fd, &namelist, scandir_error_select, NULL);
+ ATF_CHECK_EQ(-1, ret);
+ ATF_CHECK_ERRNO(EBADF, ret < 0);
+ ATF_CHECK_EQ(NULL, namelist);
+}
+
ATF_TP_ADD_TCS(tp)
{
ATF_TP_ADD_TC(tp, scandir_test);
ATF_TP_ADD_TC(tp, fdscandir_test);
ATF_TP_ADD_TC(tp, scandirat_test);
ATF_TP_ADD_TC(tp, scandir_none);
+ ATF_TP_ADD_TC(tp, scandir_error);
return (atf_no_error());
}