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()); }