[RFC][PATCHES] getting rid of int *open in ->atomic_open() and friends

[RFC][PATCHES] getting rid of int *open in ->atomic_open() and friends

[Al Viro]

	->atomic_open() calling conventions are nasty; we have
two bits of state ("has file gotten past ->open()" and "have
we created a new file here") passed by reference, with rather
brittle logics in the callers.  In some error cases do_last()
et.al.  end up doing a full-blow fput() (and destroying the
struct file they've got), in some they leave that work (a
trimmed-down subset of it, at that) to path_openat(), with
the aforementioned state affecting what's going on.

	It would be much easier if we stored the "has the
damn thing been past ->open()" (i.e. does it need fput()
or put_filp()) in file->f_mode.  It doesn't take a lot
of massage to do that - mostly it's a matter of leaving
all struct file freeing in error cases to path_openat()
itself and preventing the wipeout of all ->f_mode bits
in do_dentry_open().  Moreover, we can store the "has it
been created" in the same ->f_mode, killing the 'int *opened'
mess completely.  The same series gets O_DIRECT checks
properly centralized in do_dentry_open(), BTW.

	Other fun stuff possible (but not done yet) is
unification of put_filp() and fput() and hopefully more
simple rules regarding those.  Need to sort out some
things about pipe and ia64 perfmon first, though...

	It's still a work-in-progress; what I've got right
now is in vfs.git#work.open;

current shortlog:
Al Viro (13):
      drm_mode_create_lease_ioctl(): switch to filp_clone_open()
      rename filp_clone_open() to file_clone_open()
      introduce FMODE_OPENED
      get rid of 'opened' argument of finish_open()
      pull fput() on late failures into path_openat()
      switch all remaining checks for FILE_OPENED to FMODE_OPENED
      now we can fold open_check_o_direct() into do_dentry_open()
      __gfs2_lookup(), nfs_finish_open() and fuse_create_open() don't need 'opened'
      introduce FMODE_CREATED and switch to it
      IMA: don't propagate opened through the entire thing
      gfs2_create_inode() doesn't need 'opened' anymore
      get rid of 'opened' argument of ->atomic_open()
      get rid of 'opened' in path_openat() and the helpers downstream
diffstat:
 arch/ia64/kernel/perfmon.c                  |  1 +
 drivers/gpu/drm/drm_lease.c                 | 16 +-------------
 drivers/misc/cxl/api.c                      |  2 +-
 drivers/staging/lustre/lustre/llite/namei.c | 11 +++++-----
 fs/9p/vfs_inode.c                           |  7 +++---
 fs/9p/vfs_inode_dotl.c                      |  7 +++---
 fs/aio.c                                    |  3 ++-
 fs/anon_inodes.c                            |  2 +-
 fs/bad_inode.c                              |  2 +-
 fs/binfmt_misc.c                            |  2 +-
 fs/ceph/file.c                              |  7 +++---
 fs/ceph/super.h                             |  3 +--
 fs/cifs/cifsfs.h                            |  3 +--
 fs/cifs/dir.c                               |  7 +++---
 fs/fuse/dir.c                               | 10 ++++-----
 fs/gfs2/inode.c                             | 32 +++++++++++++--------------
 fs/hugetlbfs/inode.c                        |  2 +-
 fs/internal.h                               |  2 --
 fs/namei.c                                  | 82 ++++++++++++++++++++++++++++------------------------------------------
 fs/nfs/dir.c                                | 14 ++++++------
 fs/nfs/nfs4_fs.h                            |  2 +-
 fs/nfs/nfs4proc.c                           |  2 +-
 fs/nfsd/vfs.c                               |  2 +-
 fs/open.c                                   | 48 +++++++++++++++--------------------------
 fs/pipe.c                                   |  2 ++
 include/linux/fs.h                          | 10 ++++++---
 include/linux/ima.h                         |  4 ++--
 ipc/shm.c                                   |  2 +-
 mm/shmem.c                                  |  2 +-
 net/socket.c                                |  2 +-
 security/integrity/ima/ima.h                |  4 ++--
 security/integrity/ima/ima_appraise.c       |  4 ++--
 security/integrity/ima/ima_main.c           | 16 +++++++-------
 33 files changed, 135 insertions(+), 180 deletions(-)

and it's been only slightly build-tested.  Review and comments
would be welcome.

[PATCH 01/13] drm_mode_create_lease_ioctl(): switch to filp_clone_open()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

It had been open-coding that and, AFAICS, getting the error case wrong.

Cc: Keith Packard <keithp@keithp.com>
Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/gpu/drm/drm_lease.c | 16 +---------------
 fs/internal.h               |  1 -
 include/linux/fs.h          |  1 +
 3 files changed, 2 insertions(+), 16 deletions(-)

diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index d345563fdff3..ce281d651ae8 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -553,24 +553,13 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
 
 	/* Clone the lessor file to create a new file for us */
 	DRM_DEBUG_LEASE("Allocating lease file\n");
-	path_get(&lessor_file->f_path);
-	lessee_file = alloc_file(&lessor_file->f_path,
-				 lessor_file->f_mode,
-				 fops_get(lessor_file->f_inode->i_fop));
-
+	lessee_file = filp_clone_open(lessor_file);
 	if (IS_ERR(lessee_file)) {
 		ret = PTR_ERR(lessee_file);
 		goto out_lessee;
 	}
 
-	/* Initialize the new file for DRM */
-	DRM_DEBUG_LEASE("Initializing the file with %p\n", lessee_file->f_op->open);
-	ret = lessee_file->f_op->open(lessee_file->f_inode, lessee_file);
-	if (ret)
-		goto out_lessee_file;
-
 	lessee_priv = lessee_file->private_data;
-
 	/* Change the file to a master one */
 	drm_master_put(&lessee_priv->master);
 	lessee_priv->master = lessee;
@@ -588,9 +577,6 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
 	DRM_DEBUG_LEASE("drm_mode_create_lease_ioctl succeeded\n");
 	return 0;
 
-out_lessee_file:
-	fput(lessee_file);
-
 out_lessee:
 	drm_master_put(&lessee);
 
diff --git a/fs/internal.h b/fs/internal.h
index 980d005b21b4..5645b4ebf494 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -127,7 +127,6 @@ int do_fchownat(int dfd, const char __user *filename, uid_t user, gid_t group,
 
 extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const struct cred *);
-extern struct file *filp_clone_open(struct file *);
 
 /*
  * inode.c
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 760d8da1b6c7..81fe0292a7ac 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2401,6 +2401,7 @@ extern struct file *filp_open(const char *, int, umode_t);
 extern struct file *file_open_root(struct dentry *, struct vfsmount *,
 				   const char *, int, umode_t);
 extern struct file * dentry_open(const struct path *, int, const struct cred *);
+extern struct file *filp_clone_open(struct file *);
 extern int filp_close(struct file *, fl_owner_t id);
 
 extern struct filename *getname_flags(const char __user *, int, int *);
-- 
2.11.0

[PATCH 02/13] rename filp_clone_open() to file_clone_open()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

'filp' naming convention is bogus; sure, it's "file pointer", but
we generally don't do that kind of Hungarian notation.  Some of
the instances have too many callers to touch, but this one has
only two, so let's sanitize it while we can...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/gpu/drm/drm_lease.c | 2 +-
 fs/binfmt_misc.c            | 2 +-
 fs/open.c                   | 4 ++--
 include/linux/fs.h          | 2 +-
 4 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/gpu/drm/drm_lease.c b/drivers/gpu/drm/drm_lease.c
index ce281d651ae8..bb5f817baf0b 100644
--- a/drivers/gpu/drm/drm_lease.c
+++ b/drivers/gpu/drm/drm_lease.c
@@ -553,7 +553,7 @@ int drm_mode_create_lease_ioctl(struct drm_device *dev,
 
 	/* Clone the lessor file to create a new file for us */
 	DRM_DEBUG_LEASE("Allocating lease file\n");
-	lessee_file = filp_clone_open(lessor_file);
+	lessee_file = file_clone_open(lessor_file);
 	if (IS_ERR(lessee_file)) {
 		ret = PTR_ERR(lessee_file);
 		goto out_lessee;
diff --git a/fs/binfmt_misc.c b/fs/binfmt_misc.c
index a41b48f82a70..86ebd9dc9469 100644
--- a/fs/binfmt_misc.c
+++ b/fs/binfmt_misc.c
@@ -205,7 +205,7 @@ static int load_misc_binary(struct linux_binprm *bprm)
 		goto error;
 
 	if (fmt->flags & MISC_FMT_OPEN_FILE) {
-		interp_file = filp_clone_open(fmt->interp_file);
+		interp_file = file_clone_open(fmt->interp_file);
 		if (!IS_ERR(interp_file))
 			deny_write_access(interp_file);
 	} else {
diff --git a/fs/open.c b/fs/open.c
index d0e955b558ad..96ab60d52303 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -1063,7 +1063,7 @@ struct file *file_open_root(struct dentry *dentry, struct vfsmount *mnt,
 }
 EXPORT_SYMBOL(file_open_root);
 
-struct file *filp_clone_open(struct file *oldfile)
+struct file *file_clone_open(struct file *oldfile)
 {
 	struct file *file;
 	int retval;
@@ -1081,7 +1081,7 @@ struct file *filp_clone_open(struct file *oldfile)
 
 	return file;
 }
-EXPORT_SYMBOL(filp_clone_open);
+EXPORT_SYMBOL(file_clone_open);
 
 long do_sys_open(int dfd, const char __user *filename, int flags, umode_t mode)
 {
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 81fe0292a7ac..8f0b4667ebf1 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2401,7 +2401,7 @@ extern struct file *filp_open(const char *, int, umode_t);
 extern struct file *file_open_root(struct dentry *, struct vfsmount *,
 				   const char *, int, umode_t);
 extern struct file * dentry_open(const struct path *, int, const struct cred *);
-extern struct file *filp_clone_open(struct file *);
+extern struct file *file_clone_open(struct file *);
 extern int filp_close(struct file *, fl_owner_t id);
 
 extern struct filename *getname_flags(const char __user *, int, int *);
-- 
2.11.0

[PATCH 03/13] introduce FMODE_OPENED

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

basically, "is that instance set up enough for regular fput(), or
do we want put_filp() for that one".

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 arch/ia64/kernel/perfmon.c | 1 +
 drivers/misc/cxl/api.c     | 2 +-
 fs/aio.c                   | 3 ++-
 fs/anon_inodes.c           | 2 +-
 fs/hugetlbfs/inode.c       | 2 +-
 fs/open.c                  | 3 ++-
 fs/pipe.c                  | 2 ++
 include/linux/fs.h         | 3 +++
 ipc/shm.c                  | 2 +-
 mm/shmem.c                 | 2 +-
 net/socket.c               | 2 +-
 11 files changed, 16 insertions(+), 8 deletions(-)

diff --git a/arch/ia64/kernel/perfmon.c b/arch/ia64/kernel/perfmon.c
index 8fb280e33114..ae4c762321d4 100644
--- a/arch/ia64/kernel/perfmon.c
+++ b/arch/ia64/kernel/perfmon.c
@@ -2684,6 +2684,7 @@ pfm_context_create(pfm_context_t *ctx, void *arg, int count, struct pt_regs *reg
 	 * initialize soft PMU state
 	 */
 	pfm_reset_pmu_state(ctx);
+	filp->f_mode |= FMODE_OPENED;
 
 	fd_install(fd, filp);
 
diff --git a/drivers/misc/cxl/api.c b/drivers/misc/cxl/api.c
index 753b1a698fc4..0427ff3f575a 100644
--- a/drivers/misc/cxl/api.c
+++ b/drivers/misc/cxl/api.c
@@ -102,7 +102,7 @@ static struct file *cxl_getfile(const char *name,
 	path.mnt = mntget(cxl_vfs_mount);
 	d_instantiate(path.dentry, inode);
 
-	file = alloc_file(&path, OPEN_FMODE(flags), fops);
+	file = alloc_file(&path, OPEN_FMODE(flags) | FMODE_OPENED, fops);
 	if (IS_ERR(file))
 		goto err_dput;
 	file->f_flags = flags & (O_ACCMODE | O_NONBLOCK);
diff --git a/fs/aio.c b/fs/aio.c
index 49f53516eef0..bbc3fc0d0524 100644
--- a/fs/aio.c
+++ b/fs/aio.c
@@ -223,7 +223,8 @@ static struct file *aio_private_file(struct kioctx *ctx, loff_t nr_pages)
 	path.mnt = mntget(aio_mnt);
 
 	d_instantiate(path.dentry, inode);
-	file = alloc_file(&path, FMODE_READ | FMODE_WRITE, &aio_ring_fops);
+	file = alloc_file(&path, FMODE_READ | FMODE_WRITE | FMODE_OPENED,
+			  &aio_ring_fops);
 	if (IS_ERR(file)) {
 		path_put(&path);
 		return file;
diff --git a/fs/anon_inodes.c b/fs/anon_inodes.c
index 3168ee4e77f4..bf952939a1d3 100644
--- a/fs/anon_inodes.c
+++ b/fs/anon_inodes.c
@@ -102,7 +102,7 @@ struct file *anon_inode_getfile(const char *name,
 
 	d_instantiate(path.dentry, anon_inode_inode);
 
-	file = alloc_file(&path, OPEN_FMODE(flags), fops);
+	file = alloc_file(&path, OPEN_FMODE(flags) | FMODE_OPENED, fops);
 	if (IS_ERR(file))
 		goto err_dput;
 	file->f_mapping = anon_inode_inode->i_mapping;
diff --git a/fs/hugetlbfs/inode.c b/fs/hugetlbfs/inode.c
index d508c7844681..e0b8cc89169c 100644
--- a/fs/hugetlbfs/inode.c
+++ b/fs/hugetlbfs/inode.c
@@ -1375,7 +1375,7 @@ struct file *hugetlb_file_setup(const char *name, size_t size,
 	inode->i_size = size;
 	clear_nlink(inode);
 
-	file = alloc_file(&path, FMODE_WRITE | FMODE_READ,
+	file = alloc_file(&path, FMODE_WRITE | FMODE_READ | FMODE_OPENED,
 			&hugetlbfs_file_operations);
 	if (IS_ERR(file))
 		goto out_dentry; /* inode is already attached */
diff --git a/fs/open.c b/fs/open.c
index 96ab60d52303..b3b1dbf0227d 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -753,7 +753,7 @@ static int do_dentry_open(struct file *f,
 	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
 
 	if (unlikely(f->f_flags & O_PATH)) {
-		f->f_mode = FMODE_PATH;
+		f->f_mode = FMODE_PATH | FMODE_OPENED;
 		f->f_op = &empty_fops;
 		return 0;
 	}
@@ -795,6 +795,7 @@ static int do_dentry_open(struct file *f,
 		if (error)
 			goto cleanup_all;
 	}
+	f->f_mode |= FMODE_OPENED;
 	if ((f->f_mode & (FMODE_READ | FMODE_WRITE)) == FMODE_READ)
 		i_readcount_inc(inode);
 	if ((f->f_mode & FMODE_READ) &&
diff --git a/fs/pipe.c b/fs/pipe.c
index 39d6f431da83..a03622bd3491 100644
--- a/fs/pipe.c
+++ b/fs/pipe.c
@@ -772,6 +772,8 @@ int create_pipe_files(struct file **res, int flags)
 		goto err_file;
 	}
 
+	res[0]->f_mode |= FMODE_OPENED;
+	f->f_mode |= FMODE_OPENED;
 	path_get(&path);
 	res[0]->private_data = inode->i_pipe;
 	res[0]->f_flags = O_RDONLY | (flags & O_NONBLOCK);
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 8f0b4667ebf1..105f071d4732 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -147,12 +147,15 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 /* Has write method(s) */
 #define FMODE_CAN_WRITE         ((__force fmode_t)0x40000)
 
+#define FMODE_OPENED		((__force fmode_t)0x80000)
+
 /* File was opened by fanotify and shouldn't generate fanotify events */
 #define FMODE_NONOTIFY		((__force fmode_t)0x4000000)
 
 /* File is capable of returning -EAGAIN if I/O will block */
 #define FMODE_NOWAIT	((__force fmode_t)0x8000000)
 
+
 /*
  * Flag for rw_copy_check_uvector and compat_rw_copy_check_uvector
  * that indicates that they should check the contents of the iovec are
diff --git a/ipc/shm.c b/ipc/shm.c
index 3cf48988d68c..2d0a6a9f2559 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -1438,7 +1438,7 @@ long do_shmat(int shmid, char __user *shmaddr, int shmflg,
 		goto out_nattch;
 	}
 
-	file = alloc_file(&path, f_mode,
+	file = alloc_file(&path, f_mode | FMODE_OPENED,
 			  is_file_hugepages(shp->shm_file) ?
 				&shm_file_operations_huge :
 				&shm_file_operations);
diff --git a/mm/shmem.c b/mm/shmem.c
index 9d6c7e595415..80103281af8d 100644
--- a/mm/shmem.c
+++ b/mm/shmem.c
@@ -4239,7 +4239,7 @@ static struct file *__shmem_file_setup(struct vfsmount *mnt, const char *name, l
 	if (IS_ERR(res))
 		goto put_path;
 
-	res = alloc_file(&path, FMODE_WRITE | FMODE_READ,
+	res = alloc_file(&path, FMODE_WRITE | FMODE_READ | FMODE_OPENED,
 		  &shmem_file_operations);
 	if (IS_ERR(res))
 		goto put_path;
diff --git a/net/socket.c b/net/socket.c
index f10f1d947c78..00934c3fa46a 100644
--- a/net/socket.c
+++ b/net/socket.c
@@ -407,7 +407,7 @@ struct file *sock_alloc_file(struct socket *sock, int flags, const char *dname)
 
 	d_instantiate(path.dentry, SOCK_INODE(sock));
 
-	file = alloc_file(&path, FMODE_READ | FMODE_WRITE,
+	file = alloc_file(&path, FMODE_READ | FMODE_WRITE | FMODE_OPENED,
 		  &socket_file_ops);
 	if (IS_ERR(file)) {
 		/* drop dentry, keep inode for a bit */
-- 
2.11.0

[PATCH 04/13] get rid of 'opened' argument of finish_open()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

We only deal with opened & FILE_OPENED in it and it's parallel to
file->f_mode & FMODE_OPENED.  So let the callers deal with the latter
instead.  Most of the call chains go through fs/namei.c:atomic_open()
and do not look at FILE_OPENED between the return from finish_open()
and return to atomic_open(), which reduces the size of patch nicely -
we propagate to *opened in atomic_open() and that leaves us with
do_tmpfile() (where we do the same) and gfs2_atomic_open() (where we
check ->f_mode instead of *opened).

Signed-off-by Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/staging/lustre/lustre/llite/namei.c |  2 +-
 fs/9p/vfs_inode.c                           |  2 +-
 fs/9p/vfs_inode_dotl.c                      |  2 +-
 fs/ceph/file.c                              |  2 +-
 fs/cifs/dir.c                               |  2 +-
 fs/fuse/dir.c                               |  2 +-
 fs/gfs2/inode.c                             |  8 ++++----
 fs/namei.c                                  |  6 +++++-
 fs/nfs/dir.c                                |  2 +-
 fs/open.c                                   | 14 ++++----------
 include/linux/fs.h                          |  3 +--
 11 files changed, 21 insertions(+), 24 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 6c9ec462eb41..b47bda601ead 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -748,7 +748,7 @@ static int ll_atomic_open(struct inode *dir, struct dentry *dentry,
 				rc = finish_no_open(file, de);
 			} else {
 				file->private_data = it;
-				rc = finish_open(file, dentry, NULL, opened);
+				rc = finish_open(file, dentry, NULL);
 				/* We dget in ll_splice_alias. finish_open takes
 				 * care of dget for fd open.
 				 */
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 9ee534159cc6..d719d7fe92ce 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -922,7 +922,7 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		v9inode->writeback_fid = (void *) inode_fid;
 	}
 	mutex_unlock(&v9inode->v_mutex);
-	err = finish_open(file, dentry, generic_file_open, opened);
+	err = finish_open(file, dentry, generic_file_open);
 	if (err)
 		goto error;
 
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 7f6ae21a27b3..0b79872848c1 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -352,7 +352,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
 	}
 	mutex_unlock(&v9inode->v_mutex);
 	/* Since we are opening a file, assign the open fid to the file */
-	err = finish_open(file, dentry, generic_file_open, opened);
+	err = finish_open(file, dentry, generic_file_open);
 	if (err)
 		goto err_clunk_old_fid;
 	file->private_data = ofid;
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index f85040d73e3d..4f2aa53c4ff6 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -474,7 +474,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 			ceph_init_inode_acls(d_inode(dentry), &acls);
 			*opened |= FILE_CREATED;
 		}
-		err = finish_open(file, dentry, ceph_open, opened);
+		err = finish_open(file, dentry, ceph_open);
 	}
 out_req:
 	if (!req->r_err && req->r_target_inode)
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 81ba6e0d88d8..ca8fc209af6d 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -541,7 +541,7 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
 	if ((oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
 		*opened |= FILE_CREATED;
 
-	rc = finish_open(file, direntry, generic_file_open, opened);
+	rc = finish_open(file, direntry, generic_file_open);
 	if (rc) {
 		if (server->ops->close)
 			server->ops->close(xid, tcon, &fid);
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 24967382a7b1..18952cd38772 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -469,7 +469,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 	d_instantiate(entry, inode);
 	fuse_change_entry_timeout(entry, &outentry);
 	fuse_invalidate_attr(dir);
-	err = finish_open(file, entry, generic_file_open, opened);
+	err = finish_open(file, entry, generic_file_open);
 	if (err) {
 		fuse_sync_release(ff, flags);
 	} else {
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 8700eb815638..7bb236b99598 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -626,7 +626,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 		error = 0;
 		if (file) {
 			if (S_ISREG(inode->i_mode))
-				error = finish_open(file, dentry, gfs2_open_common, opened);
+				error = finish_open(file, dentry, gfs2_open_common);
 			else
 				error = finish_no_open(file, NULL);
 		}
@@ -768,7 +768,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 	d_instantiate(dentry, inode);
 	if (file) {
 		*opened |= FILE_CREATED;
-		error = finish_open(file, dentry, gfs2_open_common, opened);
+		error = finish_open(file, dentry, gfs2_open_common);
 	}
 	gfs2_glock_dq_uninit(ghs);
 	gfs2_glock_dq_uninit(ghs + 1);
@@ -866,7 +866,7 @@ static struct dentry *__gfs2_lookup(struct inode *dir, struct dentry *dentry,
 		return d;
 	}
 	if (file && S_ISREG(inode->i_mode))
-		error = finish_open(file, dentry, gfs2_open_common, opened);
+		error = finish_open(file, dentry, gfs2_open_common);
 
 	gfs2_glock_dq_uninit(&gh);
 	if (error) {
@@ -1250,7 +1250,7 @@ static int gfs2_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (d != NULL)
 		dentry = d;
 	if (d_really_is_positive(dentry)) {
-		if (!(*opened & FILE_OPENED))
+		if (!(file->f_mode & FMODE_OPENED))
 			return finish_no_open(file, d);
 		dput(d);
 		return 0;
diff --git a/fs/namei.c b/fs/namei.c
index 4eb916996345..b34ead1b8790 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3027,6 +3027,8 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 		 * permission here.
 		 */
 		int acc_mode = op->acc_mode;
+		if (file->f_mode & FMODE_OPENED)
+			*opened |= FILE_OPENED;
 		if (*opened & FILE_CREATED) {
 			WARN_ON(!(open_flag & O_CREAT));
 			fsnotify_create(dir, dentry);
@@ -3446,9 +3448,11 @@ static int do_tmpfile(struct nameidata *nd, unsigned flags,
 	if (error)
 		goto out2;
 	file->f_path.mnt = path.mnt;
-	error = finish_open(file, child, NULL, opened);
+	error = finish_open(file, child, NULL);
 	if (error)
 		goto out2;
+	if (file->f_mode & FMODE_OPENED)
+		*opened |= FILE_OPENED;
 	error = open_check_o_direct(file);
 	if (error)
 		fput(file);
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 73f8b43d988c..5b8f1d32d6b8 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1423,7 +1423,7 @@ static int nfs_finish_open(struct nfs_open_context *ctx,
 {
 	int err;
 
-	err = finish_open(file, dentry, do_open, opened);
+	err = finish_open(file, dentry, do_open);
 	if (err)
 		goto out;
 	if (S_ISREG(file->f_path.dentry->d_inode->i_mode))
diff --git a/fs/open.c b/fs/open.c
index b3b1dbf0227d..83e489d43526 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -742,7 +742,7 @@ static int do_dentry_open(struct file *f,
 	static const struct file_operations empty_fops = {};
 	int error;
 
-	f->f_mode = OPEN_FMODE(f->f_flags) | FMODE_LSEEK |
+	f->f_mode |= OPEN_FMODE(f->f_flags) | FMODE_LSEEK |
 				FMODE_PREAD | FMODE_PWRITE;
 
 	path_get(&f->f_path);
@@ -848,19 +848,13 @@ static int do_dentry_open(struct file *f,
  * Returns zero on success or -errno if the open failed.
  */
 int finish_open(struct file *file, struct dentry *dentry,
-		int (*open)(struct inode *, struct file *),
-		int *opened)
+		int (*open)(struct inode *, struct file *))
 {
-	int error;
-	BUG_ON(*opened & FILE_OPENED); /* once it's opened, it's opened */
+	BUG_ON(file->f_mode & FMODE_OPENED); /* once it's opened, it's opened */
 
 	file->f_path.dentry = dentry;
-	error = do_dentry_open(file, d_backing_inode(dentry), open,
+	return do_dentry_open(file, d_backing_inode(dentry), open,
 			       current_cred());
-	if (!error)
-		*opened |= FILE_OPENED;
-
-	return error;
 }
 EXPORT_SYMBOL(finish_open);
 
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 105f071d4732..a7c090c2b6b9 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -2417,8 +2417,7 @@ enum {
 	FILE_OPENED = 2
 };
 extern int finish_open(struct file *file, struct dentry *dentry,
-			int (*open)(struct inode *, struct file *),
-			int *opened);
+			int (*open)(struct inode *, struct file *));
 extern int finish_no_open(struct file *file, struct dentry *dentry);
 
 /* fs/ioctl.c */
-- 
2.11.0

[PATCH 05/13] pull fput() on late failures into path_openat()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/namei.c | 31 +++++++++++++++----------------
 1 file changed, 15 insertions(+), 16 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index b34ead1b8790..2abd00079b26 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3375,8 +3375,6 @@ static int do_last(struct nameidata *nd,
 	if (!error && will_truncate)
 		error = handle_truncate(file);
 out:
-	if (unlikely(error) && (*opened & FILE_OPENED))
-		fput(file);
 	if (unlikely(error > 0)) {
 		WARN_ON(1);
 		error = -EINVAL;
@@ -3454,8 +3452,6 @@ static int do_tmpfile(struct nameidata *nd, unsigned flags,
 	if (file->f_mode & FMODE_OPENED)
 		*opened |= FILE_OPENED;
 	error = open_check_o_direct(file);
-	if (error)
-		fput(file);
 out2:
 	mnt_drop_write(path.mnt);
 out:
@@ -3517,20 +3513,23 @@ static struct file *path_openat(struct nameidata *nd,
 	}
 	terminate_walk(nd);
 out2:
-	if (!(opened & FILE_OPENED)) {
-		BUG_ON(!error);
-		put_filp(file);
+	if (likely(!error)) {
+		if (likely(opened & FILE_OPENED))
+			return file;
+		WARN_ON(1);
+		error = -EINVAL;
 	}
-	if (unlikely(error)) {
-		if (error == -EOPENSTALE) {
-			if (flags & LOOKUP_RCU)
-				error = -ECHILD;
-			else
-				error = -ESTALE;
-		}
-		file = ERR_PTR(error);
+	if (opened & FILE_OPENED)
+		fput(file);
+	else
+		put_filp(file);
+	if (error == -EOPENSTALE) {
+		if (flags & LOOKUP_RCU)
+			error = -ECHILD;
+		else
+			error = -ESTALE;
 	}
-	return file;
+	return ERR_PTR(error);
 }
 
 struct file *do_filp_open(int dfd, struct filename *pathname,
-- 
2.11.0

[PATCH 06/13] switch all remaining checks for FILE_OPENED to FMODE_OPENED

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/namei.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 2abd00079b26..3034c97df757 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3027,8 +3027,6 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 		 * permission here.
 		 */
 		int acc_mode = op->acc_mode;
-		if (file->f_mode & FMODE_OPENED)
-			*opened |= FILE_OPENED;
 		if (*opened & FILE_CREATED) {
 			WARN_ON(!(open_flag & O_CREAT));
 			fsnotify_create(dir, dentry);
@@ -3363,11 +3361,10 @@ static int do_last(struct nameidata *nd,
 	error = may_open(&nd->path, acc_mode, open_flag);
 	if (error)
 		goto out;
-	BUG_ON(*opened & FILE_OPENED); /* once it's opened, it's opened */
+	BUG_ON(file->f_mode & FMODE_OPENED); /* once it's opened, it's opened */
 	error = vfs_open(&nd->path, file, current_cred());
 	if (error)
 		goto out;
-	*opened |= FILE_OPENED;
 opened:
 	error = open_check_o_direct(file);
 	if (!error)
@@ -3449,8 +3446,6 @@ static int do_tmpfile(struct nameidata *nd, unsigned flags,
 	error = finish_open(file, child, NULL);
 	if (error)
 		goto out2;
-	if (file->f_mode & FMODE_OPENED)
-		*opened |= FILE_OPENED;
 	error = open_check_o_direct(file);
 out2:
 	mnt_drop_write(path.mnt);
@@ -3492,8 +3487,6 @@ static struct file *path_openat(struct nameidata *nd,
 
 	if (unlikely(file->f_flags & O_PATH)) {
 		error = do_o_path(nd, flags, file);
-		if (!error)
-			opened |= FILE_OPENED;
 		goto out2;
 	}
 
@@ -3514,12 +3507,12 @@ static struct file *path_openat(struct nameidata *nd,
 	terminate_walk(nd);
 out2:
 	if (likely(!error)) {
-		if (likely(opened & FILE_OPENED))
+		if (likely(file->f_mode & FMODE_OPENED))
 			return file;
 		WARN_ON(1);
 		error = -EINVAL;
 	}
-	if (opened & FILE_OPENED)
+	if (file->f_mode & FMODE_OPENED)
 		fput(file);
 	else
 		put_filp(file);
-- 
2.11.0

[PATCH 07/13] now we can fold open_check_o_direct() into do_dentry_open()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/internal.h |  1 -
 fs/namei.c    |  7 +------
 fs/open.c     | 27 +++++++++------------------
 3 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/fs/internal.h b/fs/internal.h
index 5645b4ebf494..08ac9cdee1b4 100644
--- a/fs/internal.h
+++ b/fs/internal.h
@@ -125,7 +125,6 @@ int do_fchmodat(int dfd, const char __user *filename, umode_t mode);
 int do_fchownat(int dfd, const char __user *filename, uid_t user, gid_t group,
 		int flag);
 
-extern int open_check_o_direct(struct file *f);
 extern int vfs_open(const struct path *, struct file *, const struct cred *);
 
 /*
diff --git a/fs/namei.c b/fs/namei.c
index 3034c97df757..ef21c904f147 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3366,9 +3366,7 @@ static int do_last(struct nameidata *nd,
 	if (error)
 		goto out;
 opened:
-	error = open_check_o_direct(file);
-	if (!error)
-		error = ima_file_check(file, op->acc_mode, *opened);
+	error = ima_file_check(file, op->acc_mode, *opened);
 	if (!error && will_truncate)
 		error = handle_truncate(file);
 out:
@@ -3444,9 +3442,6 @@ static int do_tmpfile(struct nameidata *nd, unsigned flags,
 		goto out2;
 	file->f_path.mnt = path.mnt;
 	error = finish_open(file, child, NULL);
-	if (error)
-		goto out2;
-	error = open_check_o_direct(file);
 out2:
 	mnt_drop_write(path.mnt);
 out:
diff --git a/fs/open.c b/fs/open.c
index 83e489d43526..2724aa2e9635 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -724,16 +724,6 @@ SYSCALL_DEFINE3(fchown, unsigned int, fd, uid_t, user, gid_t, group)
 	return ksys_fchown(fd, user, group);
 }
 
-int open_check_o_direct(struct file *f)
-{
-	/* NB: we're sure to have correct a_ops only after f_op->open */
-	if (f->f_flags & O_DIRECT) {
-		if (!f->f_mapping->a_ops || !f->f_mapping->a_ops->direct_IO)
-			return -EINVAL;
-	}
-	return 0;
-}
-
 static int do_dentry_open(struct file *f,
 			  struct inode *inode,
 			  int (*open)(struct inode *, struct file *),
@@ -810,6 +800,11 @@ static int do_dentry_open(struct file *f,
 
 	file_ra_state_init(&f->f_ra, f->f_mapping->host->i_mapping);
 
+	/* NB: we're sure to have correct a_ops only after f_op->open */
+	if (f->f_flags & O_DIRECT) {
+		if (!f->f_mapping->a_ops || !f->f_mapping->a_ops->direct_IO)
+			return -EINVAL;
+	}
 	return 0;
 
 cleanup_all:
@@ -918,15 +913,11 @@ struct file *dentry_open(const struct path *path, int flags,
 	if (!IS_ERR(f)) {
 		f->f_flags = flags;
 		error = vfs_open(path, f, cred);
-		if (!error) {
-			/* from now on we need fput() to dispose of f */
-			error = open_check_o_direct(f);
-			if (error) {
+		if (error) {
+			if (f->f_mode & FMODE_OPENED)
 				fput(f);
-				f = ERR_PTR(error);
-			}
-		} else { 
-			put_filp(f);
+			else
+				put_filp(f);
 			f = ERR_PTR(error);
 		}
 	}
-- 
2.11.0

[PATCH 08/13] __gfs2_lookup(), nfs_finish_open() and fuse_create_open() don't need 'opened'

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/fuse/dir.c   | 4 ++--
 fs/gfs2/inode.c | 7 +++----
 fs/nfs/dir.c    | 5 ++---
 3 files changed, 7 insertions(+), 9 deletions(-)

diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 18952cd38772..ae40501d39c9 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -399,7 +399,7 @@ static struct dentry *fuse_lookup(struct inode *dir, struct dentry *entry,
  */
 static int fuse_create_open(struct inode *dir, struct dentry *entry,
 			    struct file *file, unsigned flags,
-			    umode_t mode, int *opened)
+			    umode_t mode)
 {
 	int err;
 	struct inode *inode;
@@ -513,7 +513,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 	if (fc->no_create)
 		goto mknod;
 
-	err = fuse_create_open(dir, entry, file, flags, mode, opened);
+	err = fuse_create_open(dir, entry, file, flags, mode);
 	if (err == -ENOSYS) {
 		fc->no_create = 1;
 		goto mknod;
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 7bb236b99598..198e07d5ecdd 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -830,14 +830,13 @@ static int gfs2_create(struct inode *dir, struct dentry *dentry,
  * @dir: The directory inode
  * @dentry: The dentry of the new inode
  * @file: File to be opened
- * @opened: atomic_open flags
  *
  *
  * Returns: errno
  */
 
 static struct dentry *__gfs2_lookup(struct inode *dir, struct dentry *dentry,
-				    struct file *file, int *opened)
+				    struct file *file)
 {
 	struct inode *inode;
 	struct dentry *d;
@@ -879,7 +878,7 @@ static struct dentry *__gfs2_lookup(struct inode *dir, struct dentry *dentry,
 static struct dentry *gfs2_lookup(struct inode *dir, struct dentry *dentry,
 				  unsigned flags)
 {
-	return __gfs2_lookup(dir, dentry, NULL, NULL);
+	return __gfs2_lookup(dir, dentry, NULL);
 }
 
 /**
@@ -1244,7 +1243,7 @@ static int gfs2_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (!d_in_lookup(dentry))
 		goto skip_lookup;
 
-	d = __gfs2_lookup(dir, dentry, file, opened);
+	d = __gfs2_lookup(dir, dentry, file);
 	if (IS_ERR(d))
 		return PTR_ERR(d);
 	if (d != NULL)
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 5b8f1d32d6b8..43dc1c66873d 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1418,8 +1418,7 @@ static int do_open(struct inode *inode, struct file *filp)
 
 static int nfs_finish_open(struct nfs_open_context *ctx,
 			   struct dentry *dentry,
-			   struct file *file, unsigned open_flags,
-			   int *opened)
+			   struct file *file, unsigned open_flags)
 {
 	int err;
 
@@ -1530,7 +1529,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		goto out;
 	}
 
-	err = nfs_finish_open(ctx, ctx->dentry, file, open_flags, opened);
+	err = nfs_finish_open(ctx, ctx->dentry, file, open_flags);
 	trace_nfs_atomic_open_exit(dir, ctx, open_flags, err);
 	put_nfs_open_context(ctx);
 out:
-- 
2.11.0

[PATCH 09/13] introduce FMODE_CREATED and switch to it

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Parallel to FILE_CREATED, goes into ->f_mode instead of *opened.
NFS is a bit of a wart here - it doesn't have file at the point
where FILE_CREATED used to be set, so we need to propagate it
there (for now).  IMA is another one (here and everywhere)...

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/staging/lustre/lustre/llite/namei.c |  2 +-
 fs/9p/vfs_inode.c                           |  2 +-
 fs/9p/vfs_inode_dotl.c                      |  2 +-
 fs/ceph/file.c                              |  2 +-
 fs/cifs/dir.c                               |  2 +-
 fs/fuse/dir.c                               |  2 +-
 fs/gfs2/inode.c                             |  2 +-
 fs/namei.c                                  | 15 ++++++++-------
 fs/nfs/dir.c                                |  5 ++++-
 fs/nfs/nfs4proc.c                           |  2 +-
 include/linux/fs.h                          |  1 +
 11 files changed, 21 insertions(+), 16 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index b47bda601ead..72376af0d4b5 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -735,7 +735,7 @@ static int ll_atomic_open(struct inode *dir, struct dentry *dentry,
 				goto out_release;
 			}
 
-			*opened |= FILE_CREATED;
+			file->f_mode |= FMODE_CREATED;
 		}
 		if (d_really_is_positive(dentry) &&
 		    it_disposition(it, DISP_OPEN_OPEN)) {
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index d719d7fe92ce..6d6af0109cf2 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -930,7 +930,7 @@ v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)
 		v9fs_cache_inode_set_cookie(d_inode(dentry), file);
 
-	*opened |= FILE_CREATED;
+	file->f_mode |= FMODE_CREATED;
 out:
 	dput(res);
 	return err;
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index 0b79872848c1..c6939b7cb18c 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -358,7 +358,7 @@ v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
 	file->private_data = ofid;
 	if (v9ses->cache == CACHE_LOOSE || v9ses->cache == CACHE_FSCACHE)
 		v9fs_cache_inode_set_cookie(inode, file);
-	*opened |= FILE_CREATED;
+	file->f_mode |= FMODE_CREATED;
 out:
 	v9fs_put_acl(dacl, pacl);
 	dput(res);
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 4f2aa53c4ff6..37ab240d4cdf 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -472,7 +472,7 @@ int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
 		dout("atomic_open finish_open on dn %p\n", dn);
 		if (req->r_op == CEPH_MDS_OP_CREATE && req->r_reply_info.has_create_ino) {
 			ceph_init_inode_acls(d_inode(dentry), &acls);
-			*opened |= FILE_CREATED;
+			file->f_mode |= FMODE_CREATED;
 		}
 		err = finish_open(file, dentry, ceph_open);
 	}
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index ca8fc209af6d..88a8178f90bb 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -539,7 +539,7 @@ cifs_atomic_open(struct inode *inode, struct dentry *direntry,
 	}
 
 	if ((oflags & (O_CREAT | O_EXCL)) == (O_CREAT | O_EXCL))
-		*opened |= FILE_CREATED;
+		file->f_mode |= FMODE_CREATED;
 
 	rc = finish_open(file, direntry, generic_file_open);
 	if (rc) {
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index ae40501d39c9..31b432a0e70b 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -508,7 +508,7 @@ static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 		goto no_open;
 
 	/* Only creates */
-	*opened |= FILE_CREATED;
+	file->f_mode |= FMODE_CREATED;
 
 	if (fc->no_create)
 		goto mknod;
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 198e07d5ecdd..146ef177982e 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -767,7 +767,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 	mark_inode_dirty(inode);
 	d_instantiate(dentry, inode);
 	if (file) {
-		*opened |= FILE_CREATED;
+		file->f_mode |= FMODE_CREATED;
 		error = finish_open(file, dentry, gfs2_open_common);
 	}
 	gfs2_glock_dq_uninit(ghs);
diff --git a/fs/namei.c b/fs/namei.c
index ef21c904f147..592d62a9f549 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3027,7 +3027,7 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 		 * permission here.
 		 */
 		int acc_mode = op->acc_mode;
-		if (*opened & FILE_CREATED) {
+		if (file->f_mode & FMODE_CREATED) {
 			WARN_ON(!(open_flag & O_CREAT));
 			fsnotify_create(dir, dentry);
 			acc_mode = 0;
@@ -3043,7 +3043,7 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 				dput(dentry);
 				dentry = file->f_path.dentry;
 			}
-			if (*opened & FILE_CREATED)
+			if (file->f_mode & FMODE_CREATED)
 				fsnotify_create(dir, dentry);
 			if (unlikely(d_is_negative(dentry))) {
 				error = -ENOENT;
@@ -3092,7 +3092,7 @@ static int lookup_open(struct nameidata *nd, struct path *path,
 	if (unlikely(IS_DEADDIR(dir_inode)))
 		return -ENOENT;
 
-	*opened &= ~FILE_CREATED;
+	file->f_mode &= ~FMODE_CREATED;
 	dentry = d_lookup(dir, &nd->last);
 	for (;;) {
 		if (!dentry) {
@@ -3177,7 +3177,7 @@ static int lookup_open(struct nameidata *nd, struct path *path,
 
 	/* Negative dentry, just create the file */
 	if (!dentry->d_inode && (open_flag & O_CREAT)) {
-		*opened |= FILE_CREATED;
+		file->f_mode |= FMODE_CREATED;
 		audit_inode_child(dir_inode, dentry, AUDIT_TYPE_CHILD_CREATE);
 		if (!dir_inode->i_op->create) {
 			error = -EACCES;
@@ -3284,7 +3284,7 @@ static int do_last(struct nameidata *nd,
 		if (error)
 			goto out;
 
-		if ((*opened & FILE_CREATED) ||
+		if ((file->f_mode & FMODE_CREATED) ||
 		    !S_ISREG(file_inode(file)->i_mode))
 			will_truncate = false;
 
@@ -3292,7 +3292,7 @@ static int do_last(struct nameidata *nd,
 		goto opened;
 	}
 
-	if (*opened & FILE_CREATED) {
+	if (file->f_mode & FMODE_CREATED) {
 		/* Don't check for write permission, don't truncate */
 		open_flag &= ~O_TRUNC;
 		will_truncate = false;
@@ -3366,7 +3366,8 @@ static int do_last(struct nameidata *nd,
 	if (error)
 		goto out;
 opened:
-	error = ima_file_check(file, op->acc_mode, *opened);
+	error = ima_file_check(file, op->acc_mode,
+				file->f_mode & FMODE_CREATED ? FILE_CREATED : 0);
 	if (!error && will_truncate)
 		error = handle_truncate(file);
 out:
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index 43dc1c66873d..bc4a045493f5 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1444,6 +1444,7 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 	struct inode *inode;
 	unsigned int lookup_flags = 0;
 	bool switched = false;
+	int created = 0;
 	int err;
 
 	/* Expect a negative dentry */
@@ -1504,7 +1505,9 @@ int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		goto out;
 
 	trace_nfs_atomic_open_enter(dir, ctx, open_flags);
-	inode = NFS_PROTO(dir)->open_context(dir, ctx, open_flags, &attr, opened);
+	inode = NFS_PROTO(dir)->open_context(dir, ctx, open_flags, &attr, &created);
+	if (created)
+		file->f_mode |= FMODE_CREATED;
 	if (IS_ERR(inode)) {
 		err = PTR_ERR(inode);
 		trace_nfs_atomic_open_exit(dir, ctx, open_flags, err);
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index b71757e85066..0504439d1600 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -2934,7 +2934,7 @@ static int _nfs4_do_open(struct inode *dir,
 		}
 	}
 	if (opened && opendata->file_created)
-		*opened |= FILE_CREATED;
+		*opened = 1;
 
 	if (pnfs_use_threshold(ctx_th, opendata->f_attr.mdsthreshold, server)) {
 		*ctx_th = opendata->f_attr.mdsthreshold;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index a7c090c2b6b9..6dafc39987fa 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -148,6 +148,7 @@ typedef int (dio_iodone_t)(struct kiocb *iocb, loff_t offset,
 #define FMODE_CAN_WRITE         ((__force fmode_t)0x40000)
 
 #define FMODE_OPENED		((__force fmode_t)0x80000)
+#define FMODE_CREATED		((__force fmode_t)0x100000)
 
 /* File was opened by fanotify and shouldn't generate fanotify events */
 #define FMODE_NONOTIFY		((__force fmode_t)0x4000000)
-- 
2.11.0

[PATCH 10/13] IMA: don't propagate opened through the entire thing

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

just check ->f_mode in ima_appraise_measurement()

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/namei.c                            |  3 +--
 fs/nfsd/vfs.c                         |  2 +-
 include/linux/ima.h                   |  4 ++--
 security/integrity/ima/ima.h          |  4 ++--
 security/integrity/ima/ima_appraise.c |  4 ++--
 security/integrity/ima/ima_main.c     | 16 ++++++++--------
 6 files changed, 16 insertions(+), 17 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 592d62a9f549..ce71a69653b1 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3366,8 +3366,7 @@ static int do_last(struct nameidata *nd,
 	if (error)
 		goto out;
 opened:
-	error = ima_file_check(file, op->acc_mode,
-				file->f_mode & FMODE_CREATED ? FILE_CREATED : 0);
+	error = ima_file_check(file, op->acc_mode);
 	if (!error && will_truncate)
 		error = handle_truncate(file);
 out:
diff --git a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c
index b0555d7d8200..55a099e47ba2 100644
--- a/fs/nfsd/vfs.c
+++ b/fs/nfsd/vfs.c
@@ -763,7 +763,7 @@ nfsd_open(struct svc_rqst *rqstp, struct svc_fh *fhp, umode_t type,
 		goto out_nfserr;
 	}
 
-	host_err = ima_file_check(file, may_flags, 0);
+	host_err = ima_file_check(file, may_flags);
 	if (host_err) {
 		fput(file);
 		goto out_nfserr;
diff --git a/include/linux/ima.h b/include/linux/ima.h
index 0e4647e0eb60..d9ba3fc363b7 100644
--- a/include/linux/ima.h
+++ b/include/linux/ima.h
@@ -16,7 +16,7 @@ struct linux_binprm;
 
 #ifdef CONFIG_IMA
 extern int ima_bprm_check(struct linux_binprm *bprm);
-extern int ima_file_check(struct file *file, int mask, int opened);
+extern int ima_file_check(struct file *file, int mask);
 extern void ima_file_free(struct file *file);
 extern int ima_file_mmap(struct file *file, unsigned long prot);
 extern int ima_read_file(struct file *file, enum kernel_read_file_id id);
@@ -34,7 +34,7 @@ static inline int ima_bprm_check(struct linux_binprm *bprm)
 	return 0;
 }
 
-static inline int ima_file_check(struct file *file, int mask, int opened)
+static inline int ima_file_check(struct file *file, int mask)
 {
 	return 0;
 }
diff --git a/security/integrity/ima/ima.h b/security/integrity/ima/ima.h
index 35fe91aa1fc9..2bd42d03910a 100644
--- a/security/integrity/ima/ima.h
+++ b/security/integrity/ima/ima.h
@@ -239,7 +239,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 			     struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len, int opened);
+			     int xattr_len);
 int ima_must_appraise(struct inode *inode, int mask, enum ima_hooks func);
 void ima_update_xattr(struct integrity_iint_cache *iint, struct file *file);
 enum integrity_status ima_get_cache_status(struct integrity_iint_cache *iint,
@@ -255,7 +255,7 @@ static inline int ima_appraise_measurement(enum ima_hooks func,
 					   struct file *file,
 					   const unsigned char *filename,
 					   struct evm_ima_xattr_data *xattr_value,
-					   int xattr_len, int opened)
+					   int xattr_len)
 {
 	return INTEGRITY_UNKNOWN;
 }
diff --git a/security/integrity/ima/ima_appraise.c b/security/integrity/ima/ima_appraise.c
index 8bd7a0733e51..deec1804a00a 100644
--- a/security/integrity/ima/ima_appraise.c
+++ b/security/integrity/ima/ima_appraise.c
@@ -212,7 +212,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 			     struct integrity_iint_cache *iint,
 			     struct file *file, const unsigned char *filename,
 			     struct evm_ima_xattr_data *xattr_value,
-			     int xattr_len, int opened)
+			     int xattr_len)
 {
 	static const char op[] = "appraise_data";
 	const char *cause = "unknown";
@@ -231,7 +231,7 @@ int ima_appraise_measurement(enum ima_hooks func,
 		cause = iint->flags & IMA_DIGSIG_REQUIRED ?
 				"IMA-signature-required" : "missing-hash";
 		status = INTEGRITY_NOLABEL;
-		if (opened & FILE_CREATED)
+		if (file->f_mode & FMODE_CREATED)
 			iint->flags |= IMA_NEW_FILE;
 		if ((iint->flags & IMA_NEW_FILE) &&
 		    (!(iint->flags & IMA_DIGSIG_REQUIRED) ||
diff --git a/security/integrity/ima/ima_main.c b/security/integrity/ima/ima_main.c
index 74d0bd7e76d7..1a112389b29c 100644
--- a/security/integrity/ima/ima_main.c
+++ b/security/integrity/ima/ima_main.c
@@ -173,7 +173,7 @@ void ima_file_free(struct file *file)
 
 static int process_measurement(struct file *file, const struct cred *cred,
 			       u32 secid, char *buf, loff_t size, int mask,
-			       enum ima_hooks func, int opened)
+			       enum ima_hooks func)
 {
 	struct inode *inode = file_inode(file);
 	struct integrity_iint_cache *iint = NULL;
@@ -299,7 +299,7 @@ static int process_measurement(struct file *file, const struct cred *cred,
 	if (rc == 0 && (action & IMA_APPRAISE_SUBMASK)) {
 		inode_lock(inode);
 		rc = ima_appraise_measurement(func, iint, file, pathname,
-					      xattr_value, xattr_len, opened);
+					      xattr_value, xattr_len);
 		inode_unlock(inode);
 	}
 	if (action & IMA_AUDIT)
@@ -343,7 +343,7 @@ int ima_file_mmap(struct file *file, unsigned long prot)
 	if (file && (prot & PROT_EXEC)) {
 		security_task_getsecid(current, &secid);
 		return process_measurement(file, current_cred(), secid, NULL,
-					   0, MAY_EXEC, MMAP_CHECK, 0);
+					   0, MAY_EXEC, MMAP_CHECK);
 	}
 
 	return 0;
@@ -369,13 +369,13 @@ int ima_bprm_check(struct linux_binprm *bprm)
 
 	security_task_getsecid(current, &secid);
 	ret = process_measurement(bprm->file, current_cred(), secid, NULL, 0,
-				  MAY_EXEC, BPRM_CHECK, 0);
+				  MAY_EXEC, BPRM_CHECK);
 	if (ret)
 		return ret;
 
 	security_cred_getsecid(bprm->cred, &secid);
 	return process_measurement(bprm->file, bprm->cred, secid, NULL, 0,
-				   MAY_EXEC, CREDS_CHECK, 0);
+				   MAY_EXEC, CREDS_CHECK);
 }
 
 /**
@@ -388,14 +388,14 @@ int ima_bprm_check(struct linux_binprm *bprm)
  * On success return 0.  On integrity appraisal error, assuming the file
  * is in policy and IMA-appraisal is in enforcing mode, return -EACCES.
  */
-int ima_file_check(struct file *file, int mask, int opened)
+int ima_file_check(struct file *file, int mask)
 {
 	u32 secid;
 
 	security_task_getsecid(current, &secid);
 	return process_measurement(file, current_cred(), secid, NULL, 0,
 				   mask & (MAY_READ | MAY_WRITE | MAY_EXEC |
-					   MAY_APPEND), FILE_CHECK, opened);
+					   MAY_APPEND), FILE_CHECK);
 }
 EXPORT_SYMBOL_GPL(ima_file_check);
 
@@ -497,7 +497,7 @@ int ima_post_read_file(struct file *file, void *buf, loff_t size,
 	func = read_idmap[read_id] ?: FILE_CHECK;
 	security_task_getsecid(current, &secid);
 	return process_measurement(file, current_cred(), secid, buf, size,
-				   MAY_READ, func, 0);
+				   MAY_READ, func);
 }
 
 static int __init init_ima(void)
-- 
2.11.0

[PATCH 11/13] gfs2_create_inode() doesn't need 'opened' anymore

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/gfs2/inode.c | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 146ef177982e..9cb09f63d831 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -580,7 +580,7 @@ static int gfs2_initxattrs(struct inode *inode, const struct xattr *xattr_array,
 static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 			     struct file *file,
 			     umode_t mode, dev_t dev, const char *symname,
-			     unsigned int size, int excl, int *opened)
+			     unsigned int size, int excl)
 {
 	const struct qstr *name = &dentry->d_name;
 	struct posix_acl *default_acl, *acl;
@@ -822,7 +822,7 @@ static int gfs2_create_inode(struct inode *dir, struct dentry *dentry,
 static int gfs2_create(struct inode *dir, struct dentry *dentry,
 		       umode_t mode, bool excl)
 {
-	return gfs2_create_inode(dir, dentry, NULL, S_IFREG | mode, 0, NULL, 0, excl, NULL);
+	return gfs2_create_inode(dir, dentry, NULL, S_IFREG | mode, 0, NULL, 0, excl);
 }
 
 /**
@@ -1188,7 +1188,7 @@ static int gfs2_symlink(struct inode *dir, struct dentry *dentry,
 	if (size >= gfs2_max_stuffed_size(GFS2_I(dir)))
 		return -ENAMETOOLONG;
 
-	return gfs2_create_inode(dir, dentry, NULL, S_IFLNK | S_IRWXUGO, 0, symname, size, 0, NULL);
+	return gfs2_create_inode(dir, dentry, NULL, S_IFLNK | S_IRWXUGO, 0, symname, size, 0);
 }
 
 /**
@@ -1203,7 +1203,7 @@ static int gfs2_symlink(struct inode *dir, struct dentry *dentry,
 static int gfs2_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 {
 	unsigned dsize = gfs2_max_stuffed_size(GFS2_I(dir));
-	return gfs2_create_inode(dir, dentry, NULL, S_IFDIR | mode, 0, NULL, dsize, 0, NULL);
+	return gfs2_create_inode(dir, dentry, NULL, S_IFDIR | mode, 0, NULL, dsize, 0);
 }
 
 /**
@@ -1218,7 +1218,7 @@ static int gfs2_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode)
 static int gfs2_mknod(struct inode *dir, struct dentry *dentry, umode_t mode,
 		      dev_t dev)
 {
-	return gfs2_create_inode(dir, dentry, NULL, mode, dev, NULL, 0, 0, NULL);
+	return gfs2_create_inode(dir, dentry, NULL, mode, dev, NULL, 0, 0);
 }
 
 /**
@@ -1261,7 +1261,7 @@ static int gfs2_atomic_open(struct inode *dir, struct dentry *dentry,
 	if (!(flags & O_CREAT))
 		return -ENOENT;
 
-	return gfs2_create_inode(dir, dentry, file, S_IFREG | mode, 0, NULL, 0, excl, opened);
+	return gfs2_create_inode(dir, dentry, file, S_IFREG | mode, 0, NULL, 0, excl);
 }
 
 /*
-- 
2.11.0

[PATCH 12/13] get rid of 'opened' argument of ->atomic_open()

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

not used anymore

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 drivers/staging/lustre/lustre/llite/namei.c | 7 +++----
 fs/9p/vfs_inode.c                           | 3 +--
 fs/9p/vfs_inode_dotl.c                      | 3 +--
 fs/bad_inode.c                              | 2 +-
 fs/ceph/file.c                              | 3 +--
 fs/ceph/super.h                             | 3 +--
 fs/cifs/cifsfs.h                            | 3 +--
 fs/cifs/dir.c                               | 3 +--
 fs/fuse/dir.c                               | 2 +-
 fs/gfs2/inode.c                             | 3 +--
 fs/namei.c                                  | 3 +--
 fs/nfs/dir.c                                | 2 +-
 fs/nfs/nfs4_fs.h                            | 2 +-
 include/linux/fs.h                          | 2 +-
 14 files changed, 16 insertions(+), 25 deletions(-)

diff --git a/drivers/staging/lustre/lustre/llite/namei.c b/drivers/staging/lustre/lustre/llite/namei.c
index 72376af0d4b5..6de32ef466b1 100644
--- a/drivers/staging/lustre/lustre/llite/namei.c
+++ b/drivers/staging/lustre/lustre/llite/namei.c
@@ -678,15 +678,14 @@ static struct dentry *ll_lookup_nd(struct inode *parent, struct dentry *dentry,
  */
 static int ll_atomic_open(struct inode *dir, struct dentry *dentry,
 			  struct file *file, unsigned int open_flags,
-			  umode_t mode, int *opened)
+			  umode_t mode)
 {
 	struct lookup_intent *it;
 	struct dentry *de;
 	int rc = 0;
 
-	CDEBUG(D_VFSTRACE, "VFS Op:name=%pd, dir=" DFID "(%p),file %p,open_flags %x,mode %x opened %d\n",
-	       dentry, PFID(ll_inode2fid(dir)), dir, file, open_flags, mode,
-	       *opened);
+	CDEBUG(D_VFSTRACE, "VFS Op:name=%pd, dir=" DFID "(%p),file %p,open_flags %x,mode %x\n",
+	       dentry, PFID(ll_inode2fid(dir)), dir, file, open_flags, mode);
 
 	/* Only negative dentries enter here */
 	LASSERT(!d_inode(dentry));
diff --git a/fs/9p/vfs_inode.c b/fs/9p/vfs_inode.c
index 6d6af0109cf2..228bc15311d8 100644
--- a/fs/9p/vfs_inode.c
+++ b/fs/9p/vfs_inode.c
@@ -864,8 +864,7 @@ struct dentry *v9fs_vfs_lookup(struct inode *dir, struct dentry *dentry,
 
 static int
 v9fs_vfs_atomic_open(struct inode *dir, struct dentry *dentry,
-		     struct file *file, unsigned flags, umode_t mode,
-		     int *opened)
+		     struct file *file, unsigned flags, umode_t mode)
 {
 	int err;
 	u32 perm;
diff --git a/fs/9p/vfs_inode_dotl.c b/fs/9p/vfs_inode_dotl.c
index c6939b7cb18c..4823e1c46999 100644
--- a/fs/9p/vfs_inode_dotl.c
+++ b/fs/9p/vfs_inode_dotl.c
@@ -241,8 +241,7 @@ v9fs_vfs_create_dotl(struct inode *dir, struct dentry *dentry, umode_t omode,
 
 static int
 v9fs_vfs_atomic_open_dotl(struct inode *dir, struct dentry *dentry,
-			  struct file *file, unsigned flags, umode_t omode,
-			  int *opened)
+			  struct file *file, unsigned flags, umode_t omode)
 {
 	int err = 0;
 	kgid_t gid;
diff --git a/fs/bad_inode.c b/fs/bad_inode.c
index 213b51dbbb60..60f839611c48 100644
--- a/fs/bad_inode.c
+++ b/fs/bad_inode.c
@@ -134,7 +134,7 @@ static int bad_inode_update_time(struct inode *inode, struct timespec *time,
 
 static int bad_inode_atomic_open(struct inode *inode, struct dentry *dentry,
 				 struct file *file, unsigned int open_flag,
-				 umode_t create_mode, int *opened)
+				 umode_t create_mode)
 {
 	return -EIO;
 }
diff --git a/fs/ceph/file.c b/fs/ceph/file.c
index 37ab240d4cdf..8ad1602ea97f 100644
--- a/fs/ceph/file.c
+++ b/fs/ceph/file.c
@@ -394,8 +394,7 @@ int ceph_open(struct inode *inode, struct file *file)
  * file or symlink, return 1 so the VFS can retry.
  */
 int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
-		     struct file *file, unsigned flags, umode_t mode,
-		     int *opened)
+		     struct file *file, unsigned flags, umode_t mode)
 {
 	struct ceph_fs_client *fsc = ceph_sb_to_client(dir->i_sb);
 	struct ceph_mds_client *mdsc = fsc->mdsc;
diff --git a/fs/ceph/super.h b/fs/ceph/super.h
index a7077a0c989f..971328b99ede 100644
--- a/fs/ceph/super.h
+++ b/fs/ceph/super.h
@@ -1025,8 +1025,7 @@ extern const struct file_operations ceph_file_fops;
 extern int ceph_renew_caps(struct inode *inode);
 extern int ceph_open(struct inode *inode, struct file *file);
 extern int ceph_atomic_open(struct inode *dir, struct dentry *dentry,
-			    struct file *file, unsigned flags, umode_t mode,
-			    int *opened);
+			    struct file *file, unsigned flags, umode_t mode);
 extern int ceph_release(struct inode *inode, struct file *filp);
 extern void ceph_fill_inline_data(struct inode *inode, struct page *locked_page,
 				  char *data, size_t len);
diff --git a/fs/cifs/cifsfs.h b/fs/cifs/cifsfs.h
index 013ba2aed8d9..a1a09caa65a2 100644
--- a/fs/cifs/cifsfs.h
+++ b/fs/cifs/cifsfs.h
@@ -65,8 +65,7 @@ extern struct inode *cifs_root_iget(struct super_block *);
 extern int cifs_create(struct inode *, struct dentry *, umode_t,
 		       bool excl);
 extern int cifs_atomic_open(struct inode *, struct dentry *,
-			    struct file *, unsigned, umode_t,
-			    int *);
+			    struct file *, unsigned, umode_t);
 extern struct dentry *cifs_lookup(struct inode *, struct dentry *,
 				  unsigned int);
 extern int cifs_unlink(struct inode *dir, struct dentry *dentry);
diff --git a/fs/cifs/dir.c b/fs/cifs/dir.c
index 88a8178f90bb..6d55f666113b 100644
--- a/fs/cifs/dir.c
+++ b/fs/cifs/dir.c
@@ -465,8 +465,7 @@ cifs_do_create(struct inode *inode, struct dentry *direntry, unsigned int xid,
 
 int
 cifs_atomic_open(struct inode *inode, struct dentry *direntry,
-		 struct file *file, unsigned oflags, umode_t mode,
-		 int *opened)
+		 struct file *file, unsigned oflags, umode_t mode)
 {
 	int rc;
 	unsigned int xid;
diff --git a/fs/fuse/dir.c b/fs/fuse/dir.c
index 31b432a0e70b..c9e1b532f1ff 100644
--- a/fs/fuse/dir.c
+++ b/fs/fuse/dir.c
@@ -489,7 +489,7 @@ static int fuse_create_open(struct inode *dir, struct dentry *entry,
 static int fuse_mknod(struct inode *, struct dentry *, umode_t, dev_t);
 static int fuse_atomic_open(struct inode *dir, struct dentry *entry,
 			    struct file *file, unsigned flags,
-			    umode_t mode, int *opened)
+			    umode_t mode)
 {
 	int err;
 	struct fuse_conn *fc = get_fuse_conn(dir);
diff --git a/fs/gfs2/inode.c b/fs/gfs2/inode.c
index 9cb09f63d831..78df5e3da44e 100644
--- a/fs/gfs2/inode.c
+++ b/fs/gfs2/inode.c
@@ -1228,14 +1228,13 @@ static int gfs2_mknod(struct inode *dir, struct dentry *dentry, umode_t mode,
  * @file: The proposed new struct file
  * @flags: open flags
  * @mode: File mode
- * @opened: Flag to say whether the file has been opened or not
  *
  * Returns: error code or 0 for success
  */
 
 static int gfs2_atomic_open(struct inode *dir, struct dentry *dentry,
 			    struct file *file, unsigned flags,
-			    umode_t mode, int *opened)
+			    umode_t mode)
 {
 	struct dentry *d;
 	bool excl = !!(flags & O_EXCL);
diff --git a/fs/namei.c b/fs/namei.c
index ce71a69653b1..189ead589811 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3018,8 +3018,7 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 	file->f_path.dentry = DENTRY_NOT_SET;
 	file->f_path.mnt = nd->path.mnt;
 	error = dir->i_op->atomic_open(dir, dentry, file,
-				       open_to_namei_flags(open_flag),
-				       mode, opened);
+				       open_to_namei_flags(open_flag), mode);
 	d_lookup_done(dentry);
 	if (!error) {
 		/*
diff --git a/fs/nfs/dir.c b/fs/nfs/dir.c
index bc4a045493f5..d4151843f02c 100644
--- a/fs/nfs/dir.c
+++ b/fs/nfs/dir.c
@@ -1435,7 +1435,7 @@ static int nfs_finish_open(struct nfs_open_context *ctx,
 
 int nfs_atomic_open(struct inode *dir, struct dentry *dentry,
 		    struct file *file, unsigned open_flags,
-		    umode_t mode, int *opened)
+		    umode_t mode)
 {
 	DECLARE_WAIT_QUEUE_HEAD_ONSTACK(wq);
 	struct nfs_open_context *ctx;
diff --git a/fs/nfs/nfs4_fs.h b/fs/nfs/nfs4_fs.h
index b374f680830c..562903f5e2a3 100644
--- a/fs/nfs/nfs4_fs.h
+++ b/fs/nfs/nfs4_fs.h
@@ -233,7 +233,7 @@ extern const struct dentry_operations nfs4_dentry_operations;
 
 /* dir.c */
 int nfs_atomic_open(struct inode *, struct dentry *, struct file *,
-		    unsigned, umode_t, int *);
+		    unsigned, umode_t);
 
 /* super.c */
 extern struct file_system_type nfs4_fs_type;
diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6dafc39987fa..b4af9018785f 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -1771,7 +1771,7 @@ struct inode_operations {
 	int (*update_time)(struct inode *, struct timespec *, int);
 	int (*atomic_open)(struct inode *, struct dentry *,
 			   struct file *, unsigned open_flag,
-			   umode_t create_mode, int *opened);
+			   umode_t create_mode);
 	int (*tmpfile) (struct inode *, struct dentry *, umode_t);
 	int (*set_acl)(struct inode *, struct posix_acl *, int);
 } ____cacheline_aligned;
-- 
2.11.0

[PATCH 13/13] get rid of 'opened' in path_openat() and the helpers downstream

[Al Viro]

From: Al Viro <viro@zeniv.linux.org.uk>

unused now

Signed-off-by: Al Viro <viro@zeniv.linux.org.uk>
---
 fs/namei.c | 22 ++++++++--------------
 1 file changed, 8 insertions(+), 14 deletions(-)

diff --git a/fs/namei.c b/fs/namei.c
index 189ead589811..00436749003b 100644
--- a/fs/namei.c
+++ b/fs/namei.c
@@ -3002,8 +3002,7 @@ static int may_o_create(const struct path *dir, struct dentry *dentry, umode_t m
 static int atomic_open(struct nameidata *nd, struct dentry *dentry,
 			struct path *path, struct file *file,
 			const struct open_flags *op,
-			int open_flag, umode_t mode,
-			int *opened)
+			int open_flag, umode_t mode)
 {
 	struct dentry *const DENTRY_NOT_SET = (void *) -1UL;
 	struct inode *dir =  nd->path.dentry->d_inode;
@@ -3071,14 +3070,11 @@ static int atomic_open(struct nameidata *nd, struct dentry *dentry,
  * specified then a negative dentry may be returned.
  *
  * An error code is returned otherwise.
- *
- * FILE_CREATE will be set in @*opened if the dentry was created and will be
- * cleared otherwise prior to returning.
  */
 static int lookup_open(struct nameidata *nd, struct path *path,
 			struct file *file,
 			const struct open_flags *op,
-			bool got_write, int *opened)
+			bool got_write)
 {
 	struct dentry *dir = nd->path.dentry;
 	struct inode *dir_inode = dir->d_inode;
@@ -3153,7 +3149,7 @@ static int lookup_open(struct nameidata *nd, struct path *path,
 
 	if (dir_inode->i_op->atomic_open) {
 		error = atomic_open(nd, dentry, path, file, op, open_flag,
-				    mode, opened);
+				    mode);
 		if (unlikely(error == -ENOENT) && create_error)
 			error = create_error;
 		return error;
@@ -3206,8 +3202,7 @@ static int lookup_open(struct nameidata *nd, struct path *path,
  * Handle the last step of open()
  */
 static int do_last(struct nameidata *nd,
-		   struct file *file, const struct open_flags *op,
-		   int *opened)
+		   struct file *file, const struct open_flags *op)
 {
 	struct dentry *dir = nd->path.dentry;
 	int open_flag = op->open_flag;
@@ -3273,7 +3268,7 @@ static int do_last(struct nameidata *nd,
 		inode_lock(dir->d_inode);
 	else
 		inode_lock_shared(dir->d_inode);
-	error = lookup_open(nd, &path, file, op, got_write, opened);
+	error = lookup_open(nd, &path, file, op, got_write);
 	if (open_flag & O_CREAT)
 		inode_unlock(dir->d_inode);
 	else
@@ -3418,7 +3413,7 @@ EXPORT_SYMBOL(vfs_tmpfile);
 
 static int do_tmpfile(struct nameidata *nd, unsigned flags,
 		const struct open_flags *op,
-		struct file *file, int *opened)
+		struct file *file)
 {
 	struct dentry *child;
 	struct path path;
@@ -3465,7 +3460,6 @@ static struct file *path_openat(struct nameidata *nd,
 {
 	const char *s;
 	struct file *file;
-	int opened = 0;
 	int error;
 
 	file = get_empty_filp();
@@ -3475,7 +3469,7 @@ static struct file *path_openat(struct nameidata *nd,
 	file->f_flags = op->open_flag;
 
 	if (unlikely(file->f_flags & __O_TMPFILE)) {
-		error = do_tmpfile(nd, flags, op, file, &opened);
+		error = do_tmpfile(nd, flags, op, file);
 		goto out2;
 	}
 
@@ -3490,7 +3484,7 @@ static struct file *path_openat(struct nameidata *nd,
 		return ERR_CAST(s);
 	}
 	while (!(error = link_path_walk(s, nd)) &&
-		(error = do_last(nd, file, op, &opened)) > 0) {
+		(error = do_last(nd, file, op)) > 0) {
 		nd->flags &= ~(LOOKUP_OPEN|LOOKUP_CREATE|LOOKUP_EXCL);
 		s = trailing_symlink(nd);
 		if (IS_ERR(s)) {
-- 
2.11.0

Re: [RFC][PATCHES] getting rid of int *open in ->atomic_open() and friends

[Linus Torvalds]

I'm obviously biased since I asked for this, but:

On Fri, Jun 8, 2018 at 11:48 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
>
>  33 files changed, 135 insertions(+), 180 deletions(-)

this already looks nice.

I'll go through the individual patches and see if there's anything
there that raises my hackles. Silence will mean assent in this case

           Linus

Re: [RFC][PATCHES] getting rid of int *open in ->atomic_open() and friends

[Al Viro]

On Fri, Jun 08, 2018 at 11:57:06AM -0700, Linus Torvalds wrote:
> I'm obviously biased since I asked for this, but:
> 
> On Fri, Jun 8, 2018 at 11:48 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> >  33 files changed, 135 insertions(+), 180 deletions(-)
> 
> this already looks nice.
> 
> I'll go through the individual patches and see if there's anything
> there that raises my hackles. Silence will mean assent in this case

BTW, looking through alloc_file() callers - in cxl_getfile() we have
	try to grab f_op owner
	try to grab fs_type
		if failed => err_module
	allocate an inode, set it up if successful
		if failed => err_fs
	allocate a dentry
		if failed => err_inode
	pin vfsmount
	d_instantiate
	alloc_file
		if failed => err_dput
	finish setting up
	return file
err_dput:
        drop vfsmount/dentry
err_inode:
        drop inode
err_fs:
	drop fs_type
err_module:
        drop f_op owner
	return an error

That's a double iput when we hit alloc_file failure...  There's a bunch
of callers that can be massaged into something along such lines (not
sharing that bug, though) and I wonder if we would be better off with
wrapper like

something(inode, mnt, name, fops, mode)
{
	struct qstr this = QSTR_INIT(name, strlen(name));
	struct path path;
	struct file *file;

	path.dentry = d_alloc_anon(mnt->mnt_sb, &this);
	if (!path.dentry)
		return ERR_PTR(-ENOMEM);
	path.mnt = mntget(mnt);
	d_instantiate(path.dentry, inode);
	file = alloc_file(&path, mode | FMODE_OPENED, fops);
	if (IS_ERR(file)) {
		ihold(inode);
		path_put(&path);
	}
	return file;
}

with users being
	allocate inode
		if failed => bugger off
	set inode up
	file = something(inode, mnt, name, fops, mode);
	if (IS_ERR(file))
		drop inode, bugger off
	finish setting file up

sock_alloc_file(): inode is coallocated with socket, otherwise it's
as above -
        struct file *file;
        if (!dname) {
		if (sock->sk)
			dname = sock->sk->sk_prot_creator->name;
		else
			dname = "";
        }
        file = something(SOCK_INODE(sock), sock_mnt, dname,
			�&socket_file_ops, FMODE_READ | FMODE_WRITE);
        if (IS_ERR(file)) {
                sock_release(sock);
                return file;
        }
        sock->file = file;
        file->f_flags = O_RDWR | (flags & O_NONBLOCK);
        file->private_data = sock;
	return file;

aio_private_file(): exactly that form, turns into
        struct file *file;
        struct inode *inode = alloc_anon_inode(aio_mnt->mnt_sb);
        if (IS_ERR(inode))  
                return ERR_CAST(inode);

        inode->i_mapping->a_ops = &aio_ctx_aops;
        inode->i_mapping->private_data = ctx;
        inode->i_size = PAGE_SIZE * nr_pages;

        file = something(inode, aio_mnt, "[aio]", &aio_ring_fops,
			 FMODE_READ | FMODE_WRITE);
        if (IS_ERR(file))
                iput(inode);
	else
		file->f_flags = O_RDWR;
        return file;

cxl_getfile(): after fixing the double-iput() in there, turns into
        struct file *file;
        struct inode *inode;
        int rc;

        if (fops->owner && !try_module_get(fops->owner))
                return ERR_PTR(-ENOENT);

        rc = simple_pin_fs(&cxl_fs_type, &cxl_vfs_mount, &cxl_fs_cnt);
        if (rc < 0) {
                pr_err("Cannot mount cxl pseudo filesystem: %d\n", rc);
                file = ERR_PTR(rc);
                goto err_module;
        }

        inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
        if (IS_ERR(inode)) {
                file = ERR_CAST(inode);
                goto err_fs;
        }

        file = something(inode, cxl_vfs_mount, name, fops, OPEN_FMODE(flags));
        if (IS_ERR(file)) {
		iput(inode);
		goto err_fs;
	}
	file->f_flags = flags & (O_ACCMODE | O_NONBLOCK);
	file->private_data = priv;
        return file;

err_fs:
        simple_release_fs(&cxl_vfs_mount, &cxl_fs_cnt);
err_module:
        module_put(fops->owner);
        return file;

__shmem_file_setup() - massaged into
        struct inode *inode;
        struct file *res;

        if (IS_ERR(mnt))
                return ERR_CAST(mnt);

        if (size < 0 || size > MAX_LFS_FILESIZE)
                return ERR_PTR(-EINVAL);

        if (shmem_acct_size(flags, size))
                return ERR_PTR(-ENOMEM);

        inode = shmem_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0, flags);
        if (unlikely(!inode)) {
		shmem_unacct_size(flags, size);
		return ERR_PTR(-ENOSPC);
	}

        inode->i_flags |= i_flags;
        inode->i_size = size;
        clear_nlink(inode);     /* It is unlinked */
        res = ERR_PTR(ramfs_nommu_expand_for_mapping(inode, size));
        if (!IS_ERR(res))
		res = something(inode, mnt, name, &shmem_file_operations,
				FMODE_WRITE | FMODE_READ);
        if (IS_ERR(res))
		iput(inode);
        return res;
(massage includes setting ->s_d_op to hybrid of simple_dentry_operations and
anon_ops).

hugetlb_file_setup() - massaged into
        struct file *file;
        struct inode *inode;
        struct vfsmount *mnt;
        int hstate_idx = get_hstate_idx(page_size_log);
        if (hstate_idx < 0)
                return ERR_PTR(-ENODEV);

        *user = NULL;
        mnt = hugetlbfs_vfsmount[hstate_idx];
        if (!mnt)
                return ERR_PTR(-ENOENT);

        if (creat_flags == HUGETLB_SHMFS_INODE && !can_do_hugetlb_shm()) {
                *user = current_user();
                if (user_shm_lock(size, *user)) {
                        task_lock(current);
                        pr_warn_once("%s (%d): Using mlock ulimits for SHM_HUGETLB is deprecated\n",
                                current->comm, current->pid);
                        task_unlock(current);
                } else {
                        *user = NULL;
                        return ERR_PTR(-EPERM);
                }
        }
        inode = hugetlbfs_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0);
        if (unlikely(!inode)) {
		file = ERR_PTR(-ENOSPC);
                goto out;
	}
        if (creat_flags == HUGETLB_SHMFS_INODE)
                inode->i_flags |= S_PRIVATE;
        clear_nlink(inode);
        inode->i_size = size;

        if (hugetlb_reserve_pages(inode, 0,
                        size >> huge_page_shift(hstate_inode(inode)), NULL,
                        acctflag))
		file = ERR_PTR(-ENOMEM);
	else
		file = something(inode, mnt, name, &hugetlbfs_file_operations,
				FMODE_WRITE | FMODE_READ);
        if (!IS_ERR(file))
		return file;
out:
        iput(inode);
        if (*user) {
                user_shm_unlock(size, *user);
                *user = NULL;
        }
        return file;

and the first caller of alloc_file() in create_pipe_files() also massages
into similar form.

That leaves
	* anon_inode_getfile() - converts to similar form, at the price of
ihold done slightly earlier, so that failure exit needs a (non-final, i.e.
very cheap) iput() we currently avoid.  Not a problem.
	* do_shmat() and the second alloc_file() in create_pipe_files().
Those are rather different - we *do* have an existing dentry/inode/mount
there and all we want on cleanup is path_put() to undo the path_get()
we'd done.
	* perfmon mess - _very_ different, and I wouldn't bet a dime on
correctness of failure exits there.  One of the issues is that it simulates
mmap as part of setup, so cleanup really is different.

AFAICS, there's a clear case for alloc_file() wrapper - 6 callers out of
10 get simpler with it, and the seventh is also a good candidate for the
same treatment.  Any naming ideas for that thing ("something" in the above)
would be welcome...

BTW, that's almost all callers of d_alloc_pseudo() - there is exactly one
caller not of that form (in __ns_get_path()) right now.  perfmon should
be another caller, but that might end up converted to the new wrapper...

As for put_filp()... the callers left in my local tree right now are
	* path_openat(), dentry_open(), file_clone_open() (all of the
same form - "put_filp() if it doesn't have FMODE_OPENED, fput() otherwise)
	* perfmon mess.
create_pipe_files() got converted to fput() with a bit of massage...

Re: [RFC][PATCHES] getting rid of int *open in ->atomic_open() and friends

[Al Viro]

On Fri, Jun 08, 2018 at 11:57:06AM -0700, Linus Torvalds wrote:
> I'm obviously biased since I asked for this, but:
> 
> On Fri, Jun 8, 2018 at 11:48 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> >
> >  33 files changed, 135 insertions(+), 180 deletions(-)
> 
> this already looks nice.
> 
> I'll go through the individual patches and see if there's anything
> there that raises my hackles. Silence will mean assent in this case

BTW, looking through alloc_file() callers - in cxl_getfile() we have
	try to grab f_op owner
	try to grab fs_type
		if failed => err_module
	allocate an inode, set it up if successful
		if failed => err_fs
	allocate a dentry
		if failed => err_inode
	pin vfsmount
	d_instantiate
	alloc_file
		if failed => err_dput
	finish setting up
	return file
err_dput:
        drop vfsmount/dentry
err_inode:
        drop inode
err_fs:
	drop fs_type
err_module:
        drop f_op owner
	return an error

That's a double iput when we hit alloc_file failure...  There's a bunch
of callers that can be massaged into something along such lines (not
sharing that bug, though) and I wonder if we would be better off with
wrapper like

something(inode, mnt, name, fops, mode)
{
	struct qstr this = QSTR_INIT(name, strlen(name));
	struct path path;
	struct file *file;

	path.dentry = d_alloc_anon(mnt->mnt_sb, &this);
	if (!path.dentry)
		return ERR_PTR(-ENOMEM);
	path.mnt = mntget(mnt);
	d_instantiate(path.dentry, inode);
	file = alloc_file(&path, mode | FMODE_OPENED, fops);
	if (IS_ERR(file)) {
		ihold(inode);
		path_put(&path);
	}
	return file;
}

with users being
	allocate inode
		if failed => bugger off
	set inode up
	file = something(inode, mnt, name, fops, mode);
	if (IS_ERR(file))
		drop inode, bugger off
	finish setting file up

sock_alloc_file(): inode is coallocated with socket, otherwise it's
as above -
        struct file *file;
        if (!dname) {
		if (sock->sk)
			dname = sock->sk->sk_prot_creator->name;
		else
			dname = "";
        }
        file = something(SOCK_INODE(sock), sock_mnt, dname,
			 &socket_file_ops, FMODE_READ | FMODE_WRITE);
        if (IS_ERR(file)) {
                sock_release(sock);
                return file;
        }
        sock->file = file;
        file->f_flags = O_RDWR | (flags & O_NONBLOCK);
        file->private_data = sock;
	return file;

aio_private_file(): exactly that form, turns into
        struct file *file;
        struct inode *inode = alloc_anon_inode(aio_mnt->mnt_sb);
        if (IS_ERR(inode))  
                return ERR_CAST(inode);

        inode->i_mapping->a_ops = &aio_ctx_aops;
        inode->i_mapping->private_data = ctx;
        inode->i_size = PAGE_SIZE * nr_pages;

        file = something(inode, aio_mnt, "[aio]", &aio_ring_fops,
			 FMODE_READ | FMODE_WRITE);
        if (IS_ERR(file))
                iput(inode);
	else
		file->f_flags = O_RDWR;
        return file;

cxl_getfile(): after fixing the double-iput() in there, turns into
        struct file *file;
        struct inode *inode;
        int rc;

        if (fops->owner && !try_module_get(fops->owner))
                return ERR_PTR(-ENOENT);

        rc = simple_pin_fs(&cxl_fs_type, &cxl_vfs_mount, &cxl_fs_cnt);
        if (rc < 0) {
                pr_err("Cannot mount cxl pseudo filesystem: %d\n", rc);
                file = ERR_PTR(rc);
                goto err_module;
        }

        inode = alloc_anon_inode(cxl_vfs_mount->mnt_sb);
        if (IS_ERR(inode)) {
                file = ERR_CAST(inode);
                goto err_fs;
        }

        file = something(inode, cxl_vfs_mount, name, fops, OPEN_FMODE(flags));
        if (IS_ERR(file)) {
		iput(inode);
		goto err_fs;
	}
	file->f_flags = flags & (O_ACCMODE | O_NONBLOCK);
	file->private_data = priv;
        return file;

err_fs:
        simple_release_fs(&cxl_vfs_mount, &cxl_fs_cnt);
err_module:
        module_put(fops->owner);
        return file;

__shmem_file_setup() - massaged into
        struct inode *inode;
        struct file *res;

        if (IS_ERR(mnt))
                return ERR_CAST(mnt);

        if (size < 0 || size > MAX_LFS_FILESIZE)
                return ERR_PTR(-EINVAL);

        if (shmem_acct_size(flags, size))
                return ERR_PTR(-ENOMEM);

        inode = shmem_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0, flags);
        if (unlikely(!inode)) {
		shmem_unacct_size(flags, size);
		return ERR_PTR(-ENOSPC);
	}

        inode->i_flags |= i_flags;
        inode->i_size = size;
        clear_nlink(inode);     /* It is unlinked */
        res = ERR_PTR(ramfs_nommu_expand_for_mapping(inode, size));
        if (!IS_ERR(res))
		res = something(inode, mnt, name, &shmem_file_operations,
				FMODE_WRITE | FMODE_READ);
        if (IS_ERR(res))
		iput(inode);
        return res;
(massage includes setting ->s_d_op to hybrid of simple_dentry_operations and
anon_ops).

hugetlb_file_setup() - massaged into
        struct file *file;
        struct inode *inode;
        struct vfsmount *mnt;
        int hstate_idx = get_hstate_idx(page_size_log);
        if (hstate_idx < 0)
                return ERR_PTR(-ENODEV);

        *user = NULL;
        mnt = hugetlbfs_vfsmount[hstate_idx];
        if (!mnt)
                return ERR_PTR(-ENOENT);

        if (creat_flags == HUGETLB_SHMFS_INODE && !can_do_hugetlb_shm()) {
                *user = current_user();
                if (user_shm_lock(size, *user)) {
                        task_lock(current);
                        pr_warn_once("%s (%d): Using mlock ulimits for SHM_HUGETLB is deprecated\n",
                                current->comm, current->pid);
                        task_unlock(current);
                } else {
                        *user = NULL;
                        return ERR_PTR(-EPERM);
                }
        }
        inode = hugetlbfs_get_inode(mnt->mnt_sb, NULL, S_IFREG | S_IRWXUGO, 0);
        if (unlikely(!inode)) {
		file = ERR_PTR(-ENOSPC);
                goto out;
	}
        if (creat_flags == HUGETLB_SHMFS_INODE)
                inode->i_flags |= S_PRIVATE;
        clear_nlink(inode);
        inode->i_size = size;

        if (hugetlb_reserve_pages(inode, 0,
                        size >> huge_page_shift(hstate_inode(inode)), NULL,
                        acctflag))
		file = ERR_PTR(-ENOMEM);
	else
		file = something(inode, mnt, name, &hugetlbfs_file_operations,
				FMODE_WRITE | FMODE_READ);
        if (!IS_ERR(file))
		return file;
out:
        iput(inode);
        if (*user) {
                user_shm_unlock(size, *user);
                *user = NULL;
        }
        return file;

and the first caller of alloc_file() in create_pipe_files() also massages
into similar form.

That leaves
	* anon_inode_getfile() - converts to similar form, at the price of
ihold done slightly earlier, so that failure exit needs a (non-final, i.e.
very cheap) iput() we currently avoid.  Not a problem.
	* do_shmat() and the second alloc_file() in create_pipe_files().
Those are rather different - we *do* have an existing dentry/inode/mount
there and all we want on cleanup is path_put() to undo the path_get()
we'd done.
	* perfmon mess - _very_ different, and I wouldn't bet a dime on
correctness of failure exits there.  One of the issues is that it simulates
mmap as part of setup, so cleanup really is different.

AFAICS, there's a clear case for alloc_file() wrapper - 6 callers out of
10 get simpler with it, and the seventh is also a good candidate for the
same treatment.  Any naming ideas for that thing ("something" in the above)
would be welcome...

BTW, that's almost all callers of d_alloc_pseudo() - there is exactly one
caller not of that form (in __ns_get_path()) right now.  perfmon should
be another caller, but that might end up converted to the new wrapper...

As for put_filp()... the callers left in my local tree right now are
	* path_openat(), dentry_open(), file_clone_open() (all of the
same form - "put_filp() if it doesn't have FMODE_OPENED, fput() otherwise)
	* perfmon mess.
create_pipe_files() got converted to fput() with a bit of massage...

Re: [RFC][PATCHES] getting rid of int *open in ->atomic_open() and friends

[Al Viro]

On Sat, Jun 09, 2018 at 06:10:51AM +0100, Al Viro wrote:

> That leaves
> 	* anon_inode_getfile() - converts to similar form, at the price of
> ihold done slightly earlier, so that failure exit needs a (non-final, i.e.
> very cheap) iput() we currently avoid.  Not a problem.
> 	* do_shmat() and the second alloc_file() in create_pipe_files().
> Those are rather different - we *do* have an existing dentry/inode/mount
> there and all we want on cleanup is path_put() to undo the path_get()
> we'd done.
> 	* perfmon mess - _very_ different, and I wouldn't bet a dime on
> correctness of failure exits there.  One of the issues is that it simulates
> mmap as part of setup, so cleanup really is different.
> 
> AFAICS, there's a clear case for alloc_file() wrapper - 6 callers out of
> 10 get simpler with it, and the seventh is also a good candidate for the
> same treatment.  Any naming ideas for that thing ("something" in the above)
> would be welcome...
> 
> BTW, that's almost all callers of d_alloc_pseudo() - there is exactly one
> caller not of that form (in __ns_get_path()) right now.  perfmon should
> be another caller, but that might end up converted to the new wrapper...
> 
> As for put_filp()... the callers left in my local tree right now are
> 	* path_openat(), dentry_open(), file_clone_open() (all of the
> same form - "put_filp() if it doesn't have FMODE_OPENED, fput() otherwise)
> 	* perfmon mess.
> create_pipe_files() got converted to fput() with a bit of massage...

Untested followup along those lines pushed; helper called alloc_file_pseudo()
and all but 3 callers of alloc_file() got converted to it.  perfmon is not
touched and it's becoming more and more annoying ;-/  It's also sticking
its tender bits into mm/* a lot - what it tries to do, AFAICS, is a heavily
open-coded vmalloc-backed mmap() of sorts.  It feels like there ought to be
a more idiomatic way of doing that kind of stuff...

Stephane, could you comment on the situation in there?  I realize that you
hadn't touched that thing in more than a decade, but I've no idea who else
might be familiar with that thing and it's very inconveniently special...

perfmon trouble

[Al Viro]

On Sat, Jun 09, 2018 at 04:51:08PM +0100, Al Viro wrote:

> Stephane, could you comment on the situation in there?  I realize that you
> hadn't touched that thing in more than a decade, but I've no idea who else
> might be familiar with that thing and it's very inconveniently special...

Having looked through that code... ouch.  It tries to have munmap-on-close,
of all things.  Which has interesting consequences; consider, for example,
	fd = perfctl(-1, PFM_CREATE_CONTEXT, &blah, 1);	// create a context
	....
	pid = fork();
	if (!pid) {
		execve("/usr/bin/something_suid", ...);
		...
	}

with something_suid(8) doing an explicit "close each descriptor past stdout"
loop.

PFM_CREATE_CONTEXT has created a context, mmapped its buffer (and stored
the address of that mapping in ctx->ctx_smpl_vaddr) and, having opened
an associated file, sticks it into descriptor table and returns the descriptor.

On fork/exec we have
	* descriptor table copied to child
	* all mappings copied to child and then destroyed by execve
	* execve ends up with the new binary (and libraries, etc.) mmapped
(in child)

Now, our careful suid-root binary does close(2) on its copy of descriptor.
pfm_flush() is called.  ctx->task != current, so we proceed to
        /*
         * remove virtual mapping, if any, for the calling task.
         * cannot reset ctx field until last user is calling close().
         *
         * ctx_smpl_vaddr must never be cleared because it is needed
         * by every task with access to the context
         *
         * When called from do_exit(), the mm context is gone already, therefore
         * mm is NULL, i.e., the VMA is already gone  and we do not have to
         * do anything here
         */
        if (ctx->ctx_smpl_vaddr && current->mm) {
                smpl_buf_vaddr = ctx->ctx_smpl_vaddr;
                smpl_buf_size  = ctx->ctx_smpl_size;
        }

        UNPROTECT_CTX(ctx, flags);

        /*
         * if there was a mapping, then we systematically remove it
         * at this point. Cannot be done inside critical section
         * because some VM function reenables interrupts.
         *
         */
        if (smpl_buf_vaddr) pfm_remove_smpl_mapping(smpl_buf_vaddr, smpl_buf_size);

... with the last call doing vm_munmap() on the area in question.  In the
address space of that suid-root binary, taking out whatever *it* had mapped
at that address range...

I wouldn't be surprised if that turned out to be realistically exploitable ;-/
Is there any documentation of that thing's semantics?  perfmonctl(2) doesn't
mention the mapping at all and link to HP site in the arch/ia64/kernel/perfmon.c
is 404-compliant.  Playing with archive.org brings a sourceforget reference,
but I hadn't been able to find anything ia64-related docs in there...

Re: perfmon trouble

[Christoph Hellwig]

Is perfmon even in still in use?  If not it might be a good idea to
drop it for good.

Re: perfmon trouble

[Matthew Wilcox]

On Mon, Jun 11, 2018 at 09:23:13AM -0700, Christoph Hellwig wrote:
> Is perfmon even in still in use?  If not it might be a good idea to
> drop it for good.

The problem is that even oprofile on ia64 depends on perfmon.

We have about four options, none of which are terribly appealing:

1. rm -rf arch/ia64.  Bit harsh to do this when hardware is still being
produced.

2. rm -rf arch/ia64's support for profiling completely.  Would anybody
complain?

3. Convert ia64 over to perf interface.  Who would do this work?

4. Fix the perfmon kernel and userspace to not assume this insane semantic.
Flag day, blah, blah.

Gentoo and Debian seem to be the only remaining living ports to Itanium
and at least the Debian one is very unofficial.

Re: perfmon trouble

[Linus Torvalds]

On Mon, Jun 11, 2018 at 9:49 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> The problem is that even oprofile on ia64 depends on perfmon.

Hmm? You can definitely enable ia64 support for oprofile even without perfmon.

Are you saying the end result isn't usable?

Because I'd be inclined to just remove CONFIG_PERFMON support, and see
if anybody even notices..

I'm not expecting a lot of people to do a lot of oprofile on ia64
anyway. It's a bit late to start optimizing things now.

Do people use perfmon still? Maybe. Maybe not. Perhaps we could just
mark it as broken in the Kconfig file for now, and see if somebody
says something?

               Linus

Re: perfmon trouble

[Matthew Wilcox]

On Mon, Jun 11, 2018 at 10:04:00AM -0700, Linus Torvalds wrote:
> On Mon, Jun 11, 2018 at 9:49 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > The problem is that even oprofile on ia64 depends on perfmon.
> 
> Hmm? You can definitely enable ia64 support for oprofile even without perfmon.

Oh, I think my memory is playing tricks on me.  This is my confusion, I think:

oprofile-$(CONFIG_PERFMON) += perfmon.o

so perfmon events are exposed through oprofile, but you can disable
perfmon without disabling oprofile.

> Because I'd be inclined to just remove CONFIG_PERFMON support, and see
> if anybody even notices..
> 
> I'm not expecting a lot of people to do a lot of oprofile on ia64
> anyway. It's a bit late to start optimizing things now.
> 
> Do people use perfmon still? Maybe. Maybe not. Perhaps we could just
> mark it as broken in the Kconfig file for now, and see if somebody
> says something?

That gets my vote.

Tony?  Fenghua?

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 792437d526c6..ff861420b8f5 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -455,6 +455,7 @@ config IA64_MCA_RECOVERY
 
 config PERFMON
        bool "Performance monitor support"
+       depends on BROKEN
        help
          Selects whether support for the IA-64 performance monitor hardware
          is included in the kernel.  This makes some kernel data-structures a

Re: perfmon trouble

[Matthew Wilcox]

On Mon, Jun 11, 2018 at 10:04:00AM -0700, Linus Torvalds wrote:
> On Mon, Jun 11, 2018 at 9:49 AM Matthew Wilcox <willy@infradead.org> wrote:
> >
> > The problem is that even oprofile on ia64 depends on perfmon.
> 
> Hmm? You can definitely enable ia64 support for oprofile even without perfmon.

Oh, I think my memory is playing tricks on me.  This is my confusion, I think:

oprofile-$(CONFIG_PERFMON) += perfmon.o

so perfmon events are exposed through oprofile, but you can disable
perfmon without disabling oprofile.

> Because I'd be inclined to just remove CONFIG_PERFMON support, and see
> if anybody even notices..
> 
> I'm not expecting a lot of people to do a lot of oprofile on ia64
> anyway. It's a bit late to start optimizing things now.
> 
> Do people use perfmon still? Maybe. Maybe not. Perhaps we could just
> mark it as broken in the Kconfig file for now, and see if somebody
> says something?

That gets my vote.

Tony?  Fenghua?

diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
index 792437d526c6..ff861420b8f5 100644
--- a/arch/ia64/Kconfig
+++ b/arch/ia64/Kconfig
@@ -455,6 +455,7 @@ config IA64_MCA_RECOVERY
 
 config PERFMON
        bool "Performance monitor support"
+       depends on BROKEN
        help
          Selects whether support for the IA-64 performance monitor hardware
          is included in the kernel.  This makes some kernel data-structures a

Re: perfmon trouble

[Stephane Eranian]

Hi,

On Mon, Jun 11, 2018 at 10:18 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Jun 11, 2018 at 10:04:00AM -0700, Linus Torvalds wrote:
> > On Mon, Jun 11, 2018 at 9:49 AM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > The problem is that even oprofile on ia64 depends on perfmon.
> >
> > Hmm? You can definitely enable ia64 support for oprofile even without perfmon.
>
> Oh, I think my memory is playing tricks on me.  This is my confusion, I think:
>
> oprofile-$(CONFIG_PERFMON) += perfmon.o
>
> so perfmon events are exposed through oprofile, but you can disable
> perfmon without disabling oprofile.
>
> > Because I'd be inclined to just remove CONFIG_PERFMON support, and see
> > if anybody even notices..
> >
> > I'm not expecting a lot of people to do a lot of oprofile on ia64
> > anyway. It's a bit late to start optimizing things now.
> >
> > Do people use perfmon still? Maybe. Maybe not. Perhaps we could just
> > mark it as broken in the Kconfig file for now, and see if somebody
> > says something?
>
Thanks Al for the careful analysis. If I understand correctly, the
problem seems to be that on
execve the mapping of the sampling buffer is not destroyed and then on
close, the kernel
unmaps whatever the new binary had in that address range. The fix
would be to ensure
the mmap is destroyed on execve.

My problem is that I do not have IA64 hw anymore, so whatever the fix,
I will not be able
to test this. In the meantime, I agree with Matthew, simply disable
PERFMON support.

thanks.


> That gets my vote.
>
> Tony?  Fenghua?
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index 792437d526c6..ff861420b8f5 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -455,6 +455,7 @@ config IA64_MCA_RECOVERY
>
>  config PERFMON
>         bool "Performance monitor support"
> +       depends on BROKEN
>         help
>           Selects whether support for the IA-64 performance monitor hardware
>           is included in the kernel.  This makes some kernel data-structures a
>

Re: perfmon trouble

[Stephane Eranian]

Hi,

On Mon, Jun 11, 2018 at 10:18 AM Matthew Wilcox <willy@infradead.org> wrote:
>
> On Mon, Jun 11, 2018 at 10:04:00AM -0700, Linus Torvalds wrote:
> > On Mon, Jun 11, 2018 at 9:49 AM Matthew Wilcox <willy@infradead.org> wrote:
> > >
> > > The problem is that even oprofile on ia64 depends on perfmon.
> >
> > Hmm? You can definitely enable ia64 support for oprofile even without perfmon.
>
> Oh, I think my memory is playing tricks on me.  This is my confusion, I think:
>
> oprofile-$(CONFIG_PERFMON) += perfmon.o
>
> so perfmon events are exposed through oprofile, but you can disable
> perfmon without disabling oprofile.
>
> > Because I'd be inclined to just remove CONFIG_PERFMON support, and see
> > if anybody even notices..
> >
> > I'm not expecting a lot of people to do a lot of oprofile on ia64
> > anyway. It's a bit late to start optimizing things now.
> >
> > Do people use perfmon still? Maybe. Maybe not. Perhaps we could just
> > mark it as broken in the Kconfig file for now, and see if somebody
> > says something?
>
Thanks Al for the careful analysis. If I understand correctly, the
problem seems to be that on
execve the mapping of the sampling buffer is not destroyed and then on
close, the kernel
unmaps whatever the new binary had in that address range. The fix
would be to ensure
the mmap is destroyed on execve.

My problem is that I do not have IA64 hw anymore, so whatever the fix,
I will not be able
to test this. In the meantime, I agree with Matthew, simply disable
PERFMON support.

thanks.


> That gets my vote.
>
> Tony?  Fenghua?
>
> diff --git a/arch/ia64/Kconfig b/arch/ia64/Kconfig
> index 792437d526c6..ff861420b8f5 100644
> --- a/arch/ia64/Kconfig
> +++ b/arch/ia64/Kconfig
> @@ -455,6 +455,7 @@ config IA64_MCA_RECOVERY
>
>  config PERFMON
>         bool "Performance monitor support"
> +       depends on BROKEN
>         help
>           Selects whether support for the IA-64 performance monitor hardware
>           is included in the kernel.  This makes some kernel data-structures a
>

Re: perfmon trouble

[Linus Torvalds]

On Mon, Jun 11, 2018 at 11:51 AM Stephane Eranian <eranian@google.com> wrote:
>
> In the meantime, I agree with Matthew, simply disable
> PERFMON support.

Ok, since everybody seems to agree on that, I marked it BROKEN for
now, in the hope that nobody even notices and we can just remove the
code entirely.

And if somebody *does* notice, at least we would also have a
sucker^Wtester for any possible patches to fix it.

               Linus

Re: perfmon trouble

[Linus Torvalds]

On Mon, Jun 11, 2018 at 11:51 AM Stephane Eranian <eranian@google.com> wrote:
>
> In the meantime, I agree with Matthew, simply disable
> PERFMON support.

Ok, since everybody seems to agree on that, I marked it BROKEN for
now, in the hope that nobody even notices and we can just remove the
code entirely.

And if somebody *does* notice, at least we would also have a
sucker^Wtester for any possible patches to fix it.

               Linus

Re: perfmon trouble

[Al Viro]

On Mon, Jun 11, 2018 at 11:51:34AM -0700, Stephane Eranian wrote:

> Thanks Al for the careful analysis. If I understand correctly, the
> problem seems to be that on
> execve the mapping of the sampling buffer is not destroyed and then on
> close, the kernel
> unmaps whatever the new binary had in that address range. The fix
> would be to ensure
> the mmap is destroyed on execve.

mmap *is* destroyed on execve; it's just that perfmon doesn't notice and
then tries to destroy it again on close, nevermind that it's a different
process, different mapping, etc.

> My problem is that I do not have IA64 hw anymore, so whatever the fix,
> I will not be able
> to test this. In the meantime, I agree with Matthew, simply disable
> PERFMON support.

Out of curiosity - what happened to never-went-into-mainline rewrite
of perfmon circa 2008?  perfmon2 sourceforget site has a collection
of those, up to 2.6.29; AFAICS, they did have a somewhat saner syscall
interface (no auto-mmap on creation, no auto-munmap attempts on close).
By this point it obviously would've been too late to resurrect anyway,
but I wonder what happened with it...

Re: perfmon trouble

[Al Viro]

On Mon, Jun 11, 2018 at 11:51:34AM -0700, Stephane Eranian wrote:

> Thanks Al for the careful analysis. If I understand correctly, the
> problem seems to be that on
> execve the mapping of the sampling buffer is not destroyed and then on
> close, the kernel
> unmaps whatever the new binary had in that address range. The fix
> would be to ensure
> the mmap is destroyed on execve.

mmap *is* destroyed on execve; it's just that perfmon doesn't notice and
then tries to destroy it again on close, nevermind that it's a different
process, different mapping, etc.

> My problem is that I do not have IA64 hw anymore, so whatever the fix,
> I will not be able
> to test this. In the meantime, I agree with Matthew, simply disable
> PERFMON support.

Out of curiosity - what happened to never-went-into-mainline rewrite
of perfmon circa 2008?  perfmon2 sourceforget site has a collection
of those, up to 2.6.29; AFAICS, they did have a somewhat saner syscall
interface (no auto-mmap on creation, no auto-munmap attempts on close).
By this point it obviously would've been too late to resurrect anyway,
but I wonder what happened with it...

Re: [RFC][PATCHES] getting rid of int *open in ->atomic_open() and friends

[Al Viro]

On Sat, Jun 09, 2018 at 06:10:51AM +0100, Al Viro wrote:
> On Fri, Jun 08, 2018 at 11:57:06AM -0700, Linus Torvalds wrote:
> > I'm obviously biased since I asked for this, but:
> > 
> > On Fri, Jun 8, 2018 at 11:48 AM Al Viro <viro@zeniv.linux.org.uk> wrote:
> > >
> > >  33 files changed, 135 insertions(+), 180 deletions(-)
> > 
> > this already looks nice.
> > 
> > I'll go through the individual patches and see if there's anything
> > there that raises my hackles. Silence will mean assent in this case
> 
> BTW, looking through alloc_file() callers - in cxl_getfile() we have
[snip]

> That's a double iput when we hit alloc_file failure...  There's a bunch
> of callers that can be massaged into something along such lines (not
> sharing that bug, though) and I wonder if we would be better off with
> wrapper like

... and call-by-editor strikes again - this window we've got it
cut'n'pasted into ocxlflash_getfile(), with the same double iput
in place...