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

[Pádraig Brady]

On 22/06/16 12:00, Pádraig Brady wrote:
> 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?

It'll need this on top, to better differentiate FTS_ERR and FTS_DNR,
as fts_build may be called multiple times for the same dir.

cheers,
Pádraig.

diff --git a/lib/fts.c b/lib/fts.c
index f3abd74..331d23b 100644
--- a/lib/fts.c
+++ b/lib/fts.c
@@ -1466,7 +1466,10 @@ fts_build (register FTS *sp, int type)
                 if (dp == NULL) {
                         if (errno) {
                                 cur->fts_errno = errno;
-                                cur->fts_info = nitems ? FTS_ERR : FTS_DNR;
+                                /* If we've not read any items yet, treat
+                                   the error as if we can't access the dir.  */
+                                cur->fts_info = (continue_readdir || nitems)
+                                                ? FTS_ERR : FTS_DNR;
                         }
                         break;
                 }
@@ -1628,7 +1631,8 @@ mem1:                           saved_errno = errno;

         /* If didn't find anything, return NULL. */
         if (!nitems) {
-                if (type == BREAD && cur->fts_info != FTS_DNR)
+                if (type == BREAD
+                    && cur->fts_info != FTS_DNR && cur->fts_info != FTS_ERR)
                         cur->fts_info = FTS_DP;
                 fts_lfree(head);
                 return (NULL);