Re: fts: slightly more robust

[Jim Meyering]

Eric Blake wrote:
> Jim Meyering <jim <at> meyering.net> writes:
>> I suspect that it'd be very hard to trigger these close failures,
>> but it's better to be safe.
>
> While we're visiting fts, how about this patch?  POSIX 2008 is clear that
> opendir should use O_DIRECTORY, so opendirat should probably do likewise.

There is no need for O_DIRECTORY in that function, since the only thing
it does with the resulting file descriptor is to apply fdopendir, and
fdopendir will fail with ENOTDIR when fd refers to a non-directory.

>  Do we want to obey the FIXME and make opendirat an independent module?

Sure, if there is another user.
Is there?

> Meanwhile, fts.c also has a diropen function which uses
> O_DIRECTORY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW.  Technically, the latter three are
> pointless given O_DIRECTORY (a directory is not a tty, is not a symlink, and

True, but O_DIRECTORY is not even guaranteed to be nonzero, here.
And even if it is, it may not be honored.

> does not block).  For two of the three, POSIX appears to allow the mix
> (O_NOCTTY is silently ignored; and the combination of O_DIRECTORY|O_NOFOLLOW 
> on
> a symlink must produce failure, although POSIX is ambiguous whether the 
> failure
> will be ELOOP or ENOTDIR).  But for the third, POSIX is explicit that the use
> of O_NONBLOCK on anything other than a fifo, block, or character device, the
> results are unspecified.
>
> I can see why we are passing all the flags - if there is a kernel that 
> supports
> some of the flags but not O_DIRECTORY, we have at least minimized other
> problems if we indeed attempt to open a non-directory.  But do we really want
> to be risking the unspecified behavior of O_NONBLOCK on a directory?

I think so.  That code is over 3 years old, and coreutils' remove.c
does the same thing, so far to no ill effect.

> Or should
> we ask the Austin group to clarify that O_NONBLOCK is safely ignored on
> directories?

Sure.  This appears to be established practice, and imho useful,
so it would be nice to make it official.

> And should I be adding any (or all) of these three safety flags
> to opendirat?

Yes, please.
The risk of malfunction or abuse here is very small,
but it's worthwhile to do whatever we can to minimize it.

You might as well add all four,
  O_DIRECTORY|O_NOCTTY|O_NONBLOCK|O_NOFOLLOW
on the off-chance that O_DIRECTORY makes the earlier openat
fail, and it fails with a more useful errno value than fdopendir.

> I audited for other uses of fdopendir that might be lacking a useful
> O_DIRECTORY, but didn't find any.

Good.  I've audited for the same, a few times ;-)

> In getcwd.c, the fd passed to fdopendir is
> always created via openat(fd,"..",O_RDONLY), and since ".." is already
> guaranteed to be a directory, the use of O_DIRECTORY would be redundant.  Any
> clients of savedir.c's fdsavedir are also candidates, but there weren't any
> within gnulib.