[2/3] lift grabbing path into caller of do_dentry_open()

Commit Message

Al Viro Aug. 22, 2024, 12:41 a.m. UTC

... and do that after the call.  Legitimate, since no ->open()
instance tries to modify ->f_path.  Never had been promised to
work, never had been done by any such instance, now it's clearly
forbidden.

Signed-off-by: Al Viro <[e-mail]>
---
 Documentation/filesystems/porting.rst |  9 +++++++++
 fs/open.c                             | 10 +++++++---
 2 files changed, 16 insertions(+), 3 deletions(-)

Comments

Christian Brauner Aug. 22, 2024, 7:54 a.m. UTC | #1

On Thu, Aug 22, 2024 at 01:41:01AM GMT, Al Viro wrote:
> ... and do that after the call.  Legitimate, since no ->open()
> instance tries to modify ->f_path.  Never had been promised to
> work, never had been done by any such instance, now it's clearly
> forbidden.
> 
> Signed-off-by: Al Viro <[e-mail]>
> ---

Yes, I like it!
Reviewed-by: Christian Brauner <[e-mail]>

Patch

diff --git a/Documentation/filesystems/porting.rst b/Documentation/filesystems/porting.rst
index 92bffcc6747a..34f83613ad6f 100644
--- a/Documentation/filesystems/porting.rst
+++ b/Documentation/filesystems/porting.rst
@@ -1141,3 +1141,12 @@  pointer are gone.
 
 set_blocksize() takes opened struct file instead of struct block_device now
 and it *must* be opened exclusive.
+
+
+---
+
+**mandatory**
+
+do not even think of modifying ->f_path in ->open() instance; it never had
+been expected to work and nobody had been insane enough to try it.  Now
+it is explicitly forbidden.
diff --git a/fs/open.c b/fs/open.c
index 2bda3aadfa24..0ec2e9a33856 100644
--- a/fs/open.c
+++ b/fs/open.c
@@ -912,7 +912,6 @@  static int do_dentry_open(struct file *f,
 	struct inode *inode = f->f_path.dentry->d_inode;
 	int error;
 
-	path_get(&f->f_path);
 	f->f_inode = inode;
 	f->f_mapping = inode->i_mapping;
 	f->f_wb_err = filemap_sample_wb_err(f->f_mapping);
@@ -1015,7 +1014,6 @@  static int do_dentry_open(struct file *f,
 	fops_put(f->f_op);
 	put_file_access(f);
 cleanup_file:
-	path_put(&f->f_path);
 	f->f_path.mnt = NULL;
 	f->f_path.dentry = NULL;
 	f->f_inode = NULL;
@@ -1042,10 +1040,14 @@  static int do_dentry_open(struct file *f,
 int finish_open(struct file *file, struct dentry *dentry,
 		int (*open)(struct inode *, struct file *))
 {
+	int err;
 	BUG_ON(file->f_mode & FMODE_OPENED); /* once it's opened, it's opened */
 
 	file->f_path.dentry = dentry;
-	return do_dentry_open(file, open);
+	err = do_dentry_open(file, open);
+	if (file->f_mode & FMODE_OPENED)
+		path_get(&file->f_path);
+	return err;
 }
 EXPORT_SYMBOL(finish_open);
 
@@ -1095,6 +1097,8 @@  int vfs_open(const struct path *path, struct file *file)
 		 */
 		fsnotify_open(file);
 	}
+	if (file->f_mode & FMODE_OPENED)
+		path_get(&file->f_path);
 	return ret;
 }