open_last_lookups(): lift O_EXCL|O_CREAT handling into do_open()
authorAl Viro <viro@zeniv.linux.org.uk>
Tue, 10 Mar 2020 14:13:53 +0000 (10:13 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Thu, 2 Apr 2020 05:09:30 +0000 (01:09 -0400)
Currently path_openat() has "EEXIST on O_EXCL|O_CREAT" checks done on one
of the ways out of open_last_lookups().  There are 4 cases:
1) the last component is . or ..; check is not done.
2) we had FMODE_OPENED or FMODE_CREATED set while in lookup_open();
check is not done.
3) symlink to be traversed is found; check is not done (nor
should it be)
4) everything else: check done (before complete_walk(), even).

In case (1) O_EXCL|O_CREAT ends up failing with -EISDIR - that's
open("/tmp/.", O_CREAT|O_EXCL, 0600)
Note that in the same conditions
open("/tmp", O_CREAT|O_EXCL, 0600)
would have yielded EEXIST.  Either error is allowed, switching to -EEXIST
in these cases would've been more consistent.

Case (2) is more subtle; first of all, if we have FMODE_CREATED set, the
object hadn't existed prior to the call.  The check should not be done in
such a case.  The rest is problematic, though - we have
FMODE_OPENED set (i.e. it went through ->atomic_open() and got
successfully opened there)
FMODE_CREATED is *NOT* set
O_CREAT and O_EXCL are both set.
Any such case is a bug - either we failed to set FMODE_CREATED when we
had, in fact, created an object (no such instances in the tree) or
we have opened a pre-existing file despite having had both O_CREAT and
O_EXCL passed.  One of those was, in fact caught (and fixed) while
sorting out this mess (gfs2 on cold dcache).  And in such situations
we should fail with EEXIST.

Note that for (1) and (4) FMODE_CREATED is not set - for (1) there's nothing
in handle_dots() to set it, for (4) we'd explicitly checked that.

And (1), (2) and (4) are exactly the cases when we leave the loop in
the caller, with do_open() called immediately after that loop.  IOW, we
can move the check over there, and make it

If we have O_CREAT|O_EXCL and after successful pathname resolution
FMODE_CREATED is *not* set, we must have run into a preexisting file and
should fail with EEXIST.

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
fs/namei.c

index 9f2d95aa2502db563b4f734ea4b87f7f8ae35994..2e9205599931030250e5c18fb1842b65c9fb8582 100644 (file)
@@ -3201,11 +3201,6 @@ finish_lookup:
                return res;
        }
 
-       if (unlikely((open_flag & (O_EXCL | O_CREAT)) == (O_EXCL | O_CREAT))) {
-               audit_inode(nd->name, nd->path.dentry, 0);
-               return ERR_PTR(-EEXIST);
-       }
-
        /* Why this, you ask?  _Now_ we might have grown LOOKUP_JUMPED... */
        return ERR_PTR(complete_walk(nd));
 }
@@ -3224,6 +3219,8 @@ static int do_open(struct nameidata *nd,
        if (!(file->f_mode & FMODE_CREATED))
                audit_inode(nd->name, nd->path.dentry, 0);
        if (open_flag & O_CREAT) {
+               if ((open_flag & O_EXCL) && !(file->f_mode & FMODE_CREATED))
+                       return -EEXIST;
                if (d_is_dir(nd->path.dentry))
                        return -EISDIR;
                error = may_create_in_sticky(nd->dir_mode, nd->dir_uid,