Re: [RFC] a corner case of open(2)

[Al Viro <viro@ZenIV.linux.org.uk>]

On Tue, Apr 26, 2016 at 02:41:37PM -0400, Valdis.Kletnieks@vt.edu wrote:
> On Tue, 26 Apr 2016 18:55:38 +0100, Al Viro said:
> 
> > 	It is a change of user-visible behaviour, but I would be very
> > surprised if anything broke from that change.  And it would help to simplify
> > the awful mess we have in there.
> 
> I have to admit that over the past 3 decades of working with Unix-y systems,
> there's been a number of times I've had to resort to 'od -cx /your/dir/here'
> to debug issues (/bin/ls -fi is *almost* equivalent, but doesn't show holes
> in the directory)
> 
> The biggest danger I can see is some shell script doing something like:
> 
> foobar > $dir/$targetfile
> 
> and $targetfile is unset. If we allow a program to get an open fd that refers
> to a directory, what are the semantics of various operations on that fd?

Huh?  We certainly do allow to get an open fd that refers to a directory -
how else could ls(1) possibly work?  See getdents(2) - it does use an
open file descriptor to specify the directory we operate upon.

We also do not allow opening directories for *write*, and in that case EISDIR
is the right error (and we do return it).  The corner case in question is
different:
	* O_CREAT present
	* O_EXCL absent
	* O_RDWR absent
	* O_WRONLY absent
	* pathname refers to existing directory

That's where POSIX says "just open it for read, as if O_CREAT hadn't been
there" and we fail with EISDIR.  With both O_CREAT and O_EXCL POSIX says
"fail with EEXIST" and we either do that or fail with EISDIR, depending on the
pathname details.  With either of O_RDWR and O_WRONLY POSIX says "fail with
EISDIR, O_CREAT or no O_CREAT" and that's what we do (and would certainly keep
doing so).

If you look at the code you'll see
        case S_IFDIR:  
                if (acc_mode & MAY_WRITE)
                        return -EISDIR;
in may_open() and
        error = -EISDIR;
        if ((open_flag & O_CREAT) && d_is_dir(nd->path.dentry))
                goto out;
in do_last().  The former is "can't open them rw or w", the latter - "can't
have O_CREAT on those".  With O_CREAT|O_RDWR as in your example either one
would trigger (in reality the latter will trigger first and the call of
may_open() several lines below won't be reached at all).