[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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

From: Bernhard Voelker
Subject: Re: [Bug 984910] rm fails to detect errors in readdir(3)
Date: Thu, 23 Jun 2016 09:04:56 +0200
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:38.0) Gecko/20100101 Thunderbird/38.7.0

On 06/23/2016 01:38 AM, Pádraig Brady wrote:
> On 22/06/16 23:53, Bernhard Voelker wrote:
>> On 06/22/2016 03:01 PM, Pádraig Brady wrote:
>>> 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.
>> Hmm, but rm(1) won't show the errno.
> Right, as FTS_DNR is actually passed out as intended in my patch,
> and rm will try to rmdir() in that case.
> I did consider the more aggressive FTS_ERR + FTS_STOP, though
> that would then happen when readdir() returned nothing (with errno).
> I was worried that some file systems may return NULL + ENOENT
> for empty dirs for example, which would now cause rm to start
> erroring where it might not have before.

hehe, ENOENT isn't too bad in rm(1)'s case anyway ... just kidding. ;-)
Well, that's a valid concern.  According to readdir(3p) on my system:

  Applications  wishing to check for error situations should set
  errno to 0 before calling readdir().  If errno is set to non-zero
  on return, an error occurred.


    Upon successful completion, readdir() shall return a pointer
    to an object of type struct dirent.  When an error is encountered,
    a null pointer shall be returned and errno shall be set to
    indicate the error. When the end of the directory is encountered,
    a null pointer shall be returned and errno is not changed.

So in theory using FTS_ERR + FTS_STOP seems to be safe.

>> I'm getting better results with the attached, i.e., handling the
>> readdir() failure like the other failures way down and setting the
>> FTS_STOP flag.
> Another reason I thought FTS_STOP too aggressive was it was
> currently used only for internal errors where we it didn't
> make any sense to proceed.

a good argument.  OTOH the error isn't propageted to rm() if we don't use

>> With Peter's LD_PRELOAD lib, I'm getting this:
>>   $ LD_PRELOAD=/tmp/libfake_readdir.so ~/findutils/find/find /tmp/ddd
>>   /tmp/ddd
>>   Forcing failure of readdir
>>   /home/berny/findutils/find/find: failed to read file names from \
>>   file system at or below ‘/tmp/ddd’: Too many levels of symbolic links
>>   $ LD_PRELOAD=/tmp/libfake_readdir.so ~/coreutils/src/rm -rfv /tmp/ddd
>>   Forcing failure of readdir
>>   Forcing failure of readdir
>>   /home/berny/coreutils/src/rm: fts_read failed: Too many levels of \
>>   symbolic links
>> WDYT?
> I'd modified the LD_PRELOAD code to have 2 paths.
> 1. to return NULL immediately and thus return FTS_DNR
> 2. to return NULL after a couple of iterations and thus return FTS_ERR.


> If you've objections to my assumptions I'm happy to adjust.

gnulib's FTS was copied from coreutils which in turn comes from glibc
which still looks quite similiar:
They also still don't catch readdir() errors there, so shouldn't we inform
them, too?

Thanks & have a nice day,

reply via email to

[Prev in Thread] Current Thread [Next in Thread]