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.
