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

From: Al Viro
Date: Tue Apr 26 2016 - 15:02:56 EST


On Tue, Apr 26, 2016 at 02:41:37PM -0400, Valdis.Kletnieks@xxxxxx 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).