lookup_open(): don't bother with fallbacks to lookup+create
authorAl Viro <viro@zeniv.linux.org.uk>
Thu, 12 Mar 2020 18:07:27 +0000 (14:07 -0400)
committerAl Viro <viro@zeniv.linux.org.uk>
Thu, 2 Apr 2020 05:09:31 +0000 (01:09 -0400)
We fall back to lookup+create (instead of atomic_open) in several cases:
1) we don't have write access to filesystem and O_TRUNC is
present in the flags.  It's not something we want ->atomic_open() to
see - it just might go ahead and truncate the file.  However, we can
pass it the flags sans O_TRUNC - eventually do_open() will call
handle_truncate() anyway.
2) we have O_CREAT | O_EXCL and we can't write to parent.
That's going to be an error, of course, but we want to know _which_
error should that be - might be EEXIST (if file exists), might be
EACCES or EROFS.  Simply stripping O_CREAT (and checking if we see
ENOENT) would suffice, if not for O_EXCL.  However, we used to have
->atomic_open() fully responsible for rejecting O_CREAT | O_EXCL
on existing file and just stripping O_CREAT would've disarmed
those checks.  With nothing downstream to catch the problem -
FMODE_OPENED used to be "don't bother with EEXIST checks,
->atomic_open() has done those".  Now EEXIST checks downstream
are skipped only if FMODE_CREATED is set - FMODE_OPENED alone
is not enough.  That has eliminated the need to fall back onto
lookup+create path in this case.
3) O_WRONLY or O_RDWR when we have no write access to
filesystem, with nothing else objectionable.  Fallback is
(and had always been) pointless.

IOW, we don't really need that fallback; all we need in such
cases is to trim O_TRUNC and O_CREAT properly.

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

index 1607560..61fdb77 100644 (file)
@@ -2939,9 +2939,6 @@ static struct dentry *atomic_open(struct nameidata *nd, struct dentry *dentry,
        struct inode *dir =  nd->path.dentry->d_inode;
        int error;
 
-       if (!(~open_flag & (O_EXCL | O_CREAT))) /* both O_EXCL and O_CREAT */
-               open_flag &= ~O_TRUNC;
-
        if (nd->flags & LOOKUP_DIRECTORY)
                open_flag |= O_DIRECTORY;
 
@@ -3038,32 +3035,20 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
         * Another problem is returing the "right" error value (e.g. for an
         * O_EXCL open we want to return EEXIST not EROFS).
         */
+       if (unlikely(!got_write))
+               open_flag &= ~O_TRUNC;
        if (open_flag & O_CREAT) {
+               if (open_flag & O_EXCL)
+                       open_flag &= ~O_TRUNC;
                if (!IS_POSIXACL(dir->d_inode))
                        mode &= ~current_umask();
-               if (unlikely(!got_write)) {
-                       create_error = -EROFS;
-                       open_flag &= ~O_CREAT;
-                       if (open_flag & (O_EXCL | O_TRUNC))
-                               goto no_open;
-                       /* No side effects, safe to clear O_CREAT */
-               } else {
+               if (likely(got_write))
                        create_error = may_o_create(&nd->path, dentry, mode);
-                       if (create_error) {
-                               open_flag &= ~O_CREAT;
-                               if (open_flag & O_EXCL)
-                                       goto no_open;
-                       }
-               }
-       } else if ((open_flag & (O_TRUNC|O_WRONLY|O_RDWR)) &&
-                  unlikely(!got_write)) {
-               /*
-                * No O_CREATE -> atomicity not a requirement -> fall
-                * back to lookup + open
-                */
-               goto no_open;
+               else
+                       create_error = -EROFS;
        }
-
+       if (create_error)
+               open_flag &= ~O_CREAT;
        if (dir_inode->i_op->atomic_open) {
                dentry = atomic_open(nd, dentry, file, open_flag, mode);
                if (unlikely(create_error) && dentry == ERR_PTR(-ENOENT))
@@ -3071,7 +3056,6 @@ static struct dentry *lookup_open(struct nameidata *nd, struct file *file,
                return dentry;
        }
 
-no_open:
        if (d_in_lookup(dentry)) {
                struct dentry *res = dir_inode->i_op->lookup(dir_inode, dentry,
                                                             nd->flags);