Attachment 232355 Details for Bug 262038 – Check for readdir errors in fts

[patch] Check for readdir errors in fts

fts-Check-for-readdir-errors.patch (text/plain), 6.68 KB, created by Mahmoud Abumandour on 2022-03-09 21:10:12 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Mahmoud Abumandour

Created: 2022-03-09 21:10:12 UTC

Size: 6.68 KB

patch

obsolete

>From 5fef1f9cb385ffbf925c8985ad3b57e21e1c50c7 Mon Sep 17 00:00:00 2001
>From: Mahmoud Abumandour <ma.mandourr@gmail.com>
>Date: Wed, 9 Mar 2022 21:26:26 +0200
>Subject: [PATCH] fts: Check for readdir errors
>
>---
> lib/libc/gen/fts-compat.c   | 33 ++++++++++++++++++++++++++++++---
> lib/libc/gen/fts-compat11.c | 34 +++++++++++++++++++++++++++++++---
> lib/libc/gen/fts.c          | 35 ++++++++++++++++++++++++++++++++---
> 3 files changed, 93 insertions(+), 9 deletions(-)
>
>diff --git a/lib/libc/gen/fts-compat.c b/lib/libc/gen/fts-compat.c
>index ccdd4f15905b..8eaa25bbab36 100644
>--- a/lib/libc/gen/fts-compat.c
>+++ b/lib/libc/gen/fts-compat.c
>@@ -610,6 +610,17 @@ __fts_set_clientptr_44bsd(FTS *sp, void *clientptr)
> 	sp->fts_clientptr = clientptr;
> }
> 
>+static struct dirent *
>+fts_safe_readdir(DIR *dirp, int *readdir_errno)
>+{
>+	errno = 0;
>+	if (!dirp)
>+		return (NULL);
>+	struct dirent *ret = freebsd11_readdir(dirp);
>+	*readdir_errno = errno;
>+	return (ret);
>+}
>+
> /*
>  * This is the tricky part -- do not casually change *anything* in here.  The
>  * idea is to build the linked list of entries that are used by fts_children
>@@ -634,7 +645,7 @@ fts_build(FTS *sp, int type)
> 	DIR *dirp;
> 	void *oldaddr;
> 	int cderrno, descend, len, level, maxlen, nlinks, oflag, saved_errno,
>-	    nostat, doadjust, dnamlen;
>+	    nostat, doadjust, dnamlen, readdir_errno;
> 	char *cp;
> 
> 	/* Set current node pointer. */
>@@ -738,8 +749,9 @@ fts_build(FTS *sp, int type)
> 
> 	/* Read the directory, attaching each entry to the `link' pointer. */
> 	doadjust = 0;
>+	readdir_errno = 0;
> 	for (head = tail = NULL, nitems = 0;
>-	    dirp && (dp = freebsd11_readdir(dirp));) {
>+	    (dp = fts_safe_readdir(dirp, &readdir_errno));) {
> 		dnamlen = dp->d_namlen;
> 		if (!ISSET(FTS_SEEDOT) && ISDOT(dp->d_name))
> 			continue;
>@@ -839,6 +851,20 @@ mem1:				saved_errno = errno;
> 		}
> 		++nitems;
> 	}
>+
>+	/*
>+	 * The only way to distinguish between "no more files" and readdir
>+	 * error is checking errno when readdir() returns NULL.
>+	 */
>+	if (readdir_errno) {
>+		cur->fts_errno = readdir_errno;
>+		/*
>+		 * If we've not read any items yet, treat
>+		 * the error as if we can't access the dir.
>+		 */
>+		cur->fts_info = nitems ? FTS_ERR : FTS_DNR;
>+	}
>+
> 	if (dirp)
> 		(void)closedir(dirp);
> 
>@@ -877,7 +903,8 @@ mem1:				saved_errno = errno;
> 
> 	/* If didn't find anything, return NULL. */
> 	if (!nitems) {
>-		if (type == BREAD)
>+		if (type == BREAD &&
>+		    cur->fts_info != FTS_DNR && cur->fts_info != FTS_ERR)
> 			cur->fts_info = FTS_DP;
> 		return (NULL);
> 	}
>diff --git a/lib/libc/gen/fts-compat11.c b/lib/libc/gen/fts-compat11.c
>index 288351d2008b..597e19a18a3d 100644
>--- a/lib/libc/gen/fts-compat11.c
>+++ b/lib/libc/gen/fts-compat11.c
>@@ -607,6 +607,17 @@ freebsd11_fts_set_clientptr(FTS11 *sp, void *clientptr)
> 	sp->fts_clientptr = clientptr;
> }
> 
>+static struct dirent *
>+fts_safe_readdir(DIR *dirp, int *readdir_errno)
>+{
>+	errno = 0;
>+	if (!dirp)
>+		return (NULL);
>+	struct dirent *ret = freebsd11_readdir(dirp);
>+	*readdir_errno = errno;
>+	return (ret);
>+}
>+
> /*
>  * This is the tricky part -- do not casually change *anything* in here.  The
>  * idea is to build the linked list of entries that are used by fts_children
>@@ -630,7 +641,8 @@ fts_build(FTS11 *sp, int type)
> 	DIR *dirp;
> 	void *oldaddr;
> 	char *cp;
>-	int cderrno, descend, oflag, saved_errno, nostat, doadjust;
>+	int cderrno, descend, oflag, saved_errno, nostat, doadjust,
>+	    readdir_errno;
> 	long level;
> 	long nlinks;	/* has to be signed because -1 is a magic value */
> 	size_t dnamlen, len, maxlen, nitems;
>@@ -736,8 +748,9 @@ fts_build(FTS11 *sp, int type)
> 
> 	/* Read the directory, attaching each entry to the `link' pointer. */
> 	doadjust = 0;
>+	readdir_errno = 0;
> 	for (head = tail = NULL, nitems = 0;
>-	    dirp && (dp = freebsd11_readdir(dirp));) {
>+	    (dp = fts_safe_readdir(dirp, &readdir_errno));) {
> 		dnamlen = dp->d_namlen;
> 		if (!ISSET(FTS_SEEDOT) && ISDOT(dp->d_name))
> 			continue;
>@@ -823,6 +836,20 @@ mem1:				saved_errno = errno;
> 		}
> 		++nitems;
> 	}
>+
>+	/*
>+	 * The only way to distinguish between "no more files" and readdir
>+	 * error is checking errno when readdir() returns NULL.
>+	 */
>+	if (readdir_errno) {
>+		cur->fts_errno = readdir_errno;
>+		/*
>+		 * If we've not read any items yet, treat
>+		 * the error as if we can't access the dir.
>+		 */
>+		cur->fts_info = nitems ? FTS_ERR : FTS_DNR;
>+	}
>+
> 	if (dirp)
> 		(void)closedir(dirp);
> 
>@@ -859,7 +886,8 @@ mem1:				saved_errno = errno;
> 
> 	/* If didn't find anything, return NULL. */
> 	if (!nitems) {
>-		if (type == BREAD)
>+		if (type == BREAD &&
>+		    cur->fts_info != FTS_DNR && cur->fts_info != FTS_ERR)
> 			cur->fts_info = FTS_DP;
> 		return (NULL);
> 	}
>diff --git a/lib/libc/gen/fts.c b/lib/libc/gen/fts.c
>index d0705e123775..4db3d06866c6 100644
>--- a/lib/libc/gen/fts.c
>+++ b/lib/libc/gen/fts.c
>@@ -604,6 +604,17 @@ fts_set_clientptr(FTS *sp, void *clientptr)
> 	sp->fts_clientptr = clientptr;
> }
> 
>+static struct dirent *
>+fts_safe_readdir(DIR *dirp, int *readdir_errno)
>+{
>+	errno = 0;
>+	if (!dirp)
>+		return (NULL);
>+	struct dirent *ret = readdir(dirp);
>+	*readdir_errno = errno;
>+	return (ret);
>+}
>+
> /*
>  * This is the tricky part -- do not casually change *anything* in here.  The
>  * idea is to build the linked list of entries that are used by fts_children
>@@ -627,7 +638,8 @@ fts_build(FTS *sp, int type)
> 	DIR *dirp;
> 	void *oldaddr;
> 	char *cp;
>-	int cderrno, descend, oflag, saved_errno, nostat, doadjust;
>+	int cderrno, descend, oflag, saved_errno, nostat, doadjust,
>+	    readdir_errno;
> 	long level;
> 	long nlinks;	/* has to be signed because -1 is a magic value */
> 	size_t dnamlen, len, maxlen, nitems;
>@@ -733,7 +745,9 @@ fts_build(FTS *sp, int type)
> 
> 	/* Read the directory, attaching each entry to the `link' pointer. */
> 	doadjust = 0;
>-	for (head = tail = NULL, nitems = 0; dirp && (dp = readdir(dirp));) {
>+	readdir_errno = 0;
>+	for (head = tail = NULL, nitems = 0;
>+	    (dp = fts_safe_readdir(dirp, &readdir_errno));) {
> 		dnamlen = dp->d_namlen;
> 		if (!ISSET(FTS_SEEDOT) && ISDOT(dp->d_name))
> 			continue;
>@@ -819,6 +833,20 @@ mem1:				saved_errno = errno;
> 		}
> 		++nitems;
> 	}
>+
>+	/*
>+	 * The only way to distinguish between "no more files" and readdir
>+	 * error is checking errno when readdir() returns NULL.
>+	 */
>+	if (readdir_errno) {
>+		cur->fts_errno = readdir_errno;
>+		/*
>+		 * If we've not read any items yet, treat
>+		 * the error as if we can't access the dir.
>+		 */
>+		cur->fts_info = nitems ? FTS_ERR : FTS_DNR;
>+	}
>+
> 	if (dirp)
> 		(void)closedir(dirp);
> 
>@@ -855,7 +883,8 @@ mem1:				saved_errno = errno;
> 
> 	/* If didn't find anything, return NULL. */
> 	if (!nitems) {
>-		if (type == BREAD)
>+		if (type == BREAD &&
>+		    cur->fts_info != FTS_DNR && cur->fts_info != FTS_ERR)
> 			cur->fts_info = FTS_DP;
> 		return (NULL);
> 	}
>-- 
>2.34.1
>

Actions: View | Diff

Attachments on bug 262038: 231923 | 232355 | 232470 | 232739