musl - [PATCH 2/4] scandir: don't examine errno after closedir()

Follow @Openwall on Twitter for new release announcements and other news

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <20250710185131.186-2-mailto.luca.kellermann@gmail.com>
Date: Thu, 10 Jul 2025 20:51:29 +0200
From: Luca Kellermann <mailto.luca.kellermann@...il.com>
To: musl@...ts.openwall.com
Subject: [PATCH 2/4] scandir: don't examine errno after closedir()

when closedir() set errno, scandir() misinterpreted this as a failure.
this was wrong for two reasons:

* if closedir() succeeds, errno could still have been set, e.g. by
  __aio_close().

* even if closedir() "fails", it always closes the file descriptor and
  frees memory, so there is no reason to free all directory entries
  and return from scandir() with a failure.

the following program demonstrates the issue with aio:

#include <aio.h>
#include <dirent.h>
#include <errno.h>
#include <signal.h>
#include <stdio.h>
#include <stdlib.h>
#include <unistd.h>
static void die(const char *msg)
{
	perror(msg);
	exit(EXIT_FAILURE);
}
int main(void)
{
	struct aiocb cb = {
		.aio_fildes = STDIN_FILENO,
		.aio_buf = &(char){0},
		.aio_nbytes = 1,
		.aio_sigevent.sigev_notify = SIGEV_NONE,
	};
	if (aio_read(&cb)) die("aio_read");
	struct dirent **es = 0;
	int e, n = scandir(".", &es, 0, alphasort);
	if (n == -1) die("scandir");
	while (n > 0) free(es[--n]);
	free(es);
	if (aio_suspend(&(const struct aiocb *){&cb}, 1, 0))
		die("aio_suspend");
	if ((e = aio_error(&cb)) == -1) die("aio_error");
	if (aio_return(&cb) == -1) {
		if (!e) die("aio_return");
		errno = e;
		die("read");
	}
}

$ ./scandir-aio
scandir: No such file or directory
---
 src/dirent/scandir.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/src/dirent/scandir.c b/src/dirent/scandir.c
index eead7e50..cf668535 100644
--- a/src/dirent/scandir.c
+++ b/src/dirent/scandir.c
@@ -12,7 +12,7 @@ int scandir(const char *path, struct dirent ***res,
 	DIR *d = opendir(path);
 	struct dirent *de, **names=0, **tmp;
 	size_t cnt=0, len=0;
-	int old_errno = errno;
+	int old_errno = errno, err;
 
 	if (!d) return -1;
 
@@ -33,12 +33,18 @@ int scandir(const char *path, struct dirent ***res,
 		if (!names[cnt]) break;
 		memcpy(names[cnt++], de, de->d_reclen);
 	}
+	/* closedir() might set errno via __aio_close(). It might also "fail"
+	 * (return -1 and set errno). But even then, the file descriptor is
+	 * closed and memory is freed, so there is no reason to report the
+	 * "failure" of closedir() as a failure of scandir(). */
+	err = errno;
 
 	closedir(d);
 
-	if (errno) {
+	if (err) {
 		if (names) while (cnt-->0) free(names[cnt]);
 		free(names);
+		errno = err;
 		return -1;
 	}
 	/* cmp() and caller must not observe that errno was set to 0. */
-- 
2.43.0

Confused about mailing lists and their use? Read about mailing lists on Wikipedia and check out these guidelines on proper formatting of your messages.