Re: [Bug 984910] rm fails to detect errors in readdir(3)

[Pádraig Brady]

On 21/06/16 19:58, Bernhard Voelker wrote:
> forwarding to gnulib.
> 
> Have a nice day,
> Berny
> 
> 
> -------- Forwarded Message --------
> Subject:      [Bug 984910] rm fails to detect errors in readdir(3)
> Date:         Wed, 15 Jun 2016 19:26:24 +0000
> From:         bugzilla_nore...@novell.com
> 
> Andreas Stieger changed bug 984910
>   <http://bugzilla.opensuse.org/show_bug.cgi?id=984910>
> 
> --------------------------------------
> openSUSE bug report by: Peter Benie <pjb1...@cam.ac.uk>
> 
> 
> rm, and anything else in coreutils using the fts functions, fails to
> detect errors in readdir(3). Thankfully such errors are rare, but an
> example of where they can occur is where a NFS server has a poor readdir
> cookie implementation.
> 
> The very latest upsteam coreutils uses the fts functions that are built
> into libc. In OpenSuSE 13.2 it uses its own copy of fts.
> 
> The error is in lib/fts.c:fts_read(), which calls readdir() without
> correctly checking the error return. readdir returns NULL on end-of-directory
> or on error; fts_read assumes that NULL always means end-of-directory.

That looks like a valid issue.
However don't we want to restrict this to only when errno!=0 ?
Also if setting FTS_DNR it could be inconsistent to return dir entries also?
Also fts_info may be set to FTS_DP if !nitems?
Also I'm not sure we need to save errno in this case.
How about the attached instead?

thanks!
Pádraig

diff --git a/lib/fts.c b/lib/fts.c
index bcdcff9..d022633 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1461,9 +1461,15 @@ fts_build (register FTS *sp, int type)
         while (cur->fts_dirp) {
                 bool is_dir;
                 size_t d_namelen;
+                __set_errno (0);
                 struct dirent *dp = readdir(cur->fts_dirp);
-                if (dp == NULL)
+                if (dp == NULL) {
+                        if (errno) {
+                                cur->fts_errno = errno;
+                                cur->fts_info = items ? FTS_ERR : FTS_DNR;
+                        }
                         break;
+                }
                 if (!ISSET(FTS_SEEDOT) && ISDOT(dp->d_name))
                         continue;
 
@@ -1622,7 +1628,7 @@ 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_DP;
                 fts_lfree(head);
                 return (NULL);