The branch main has been updated by des:

URL: 
https://cgit.FreeBSD.org/src/commit/?id=62e0f12f5104585b7346fee183e5c667b39ddbad

commit 62e0f12f5104585b7346fee183e5c667b39ddbad
Author:     Dag-Erling Smørgrav <d...@freebsd.org>
AuthorDate: 2025-06-26 07:37:00 +0000
Commit:     Dag-Erling Smørgrav <d...@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 <dfirb...@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());
 }

Reply via email to