Inter-revision diff: patch 15

Comparing v5 (message) to v6 (message)

--- v5
+++ v6
@@ -1,64 +1,214 @@
-Add a simple helper to retrieve the user namespace associated with the
-vfsmount of a file. Christoph correctly points out that this makes
-codepaths (e.g. ioctls) way easier to follow that would otherwise
-dereference via mnt_user_ns(file->f_path.mnt).
-
-In order to make file_user_ns() static inline we'd need to include
-mount.h in either file.h or fs.h which seems undesirable so let's simply
-not force file_user_ns() to be inline.
-
+When truncating files the vfs will verify that the caller is privileged
+over the inode. Extend it to handle idmapped mounts. If the inode is
+accessed through an idmapped mount it is mapped according to the mount's
+user namespace. Afterwards the permissions checks are identical to
+non-idmapped mounts. If the initial user namespace is passed nothing
+changes so non-idmapped mounts will see identical behavior as before.
+
+Link: https://lore.kernel.org/r/20210112220124.837960-23-christian.brauner@ubuntu.com
+Cc: Christoph Hellwig <hch@lst.de>
 Cc: David Howells <dhowells@redhat.com>
 Cc: Al Viro <viro@zeniv.linux.org.uk>
 Cc: linux-fsdevel@vger.kernel.org
-Suggested-by: Christoph Hellwig <hch@lst.de>
 Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
 ---
 /* v2 */
-patch not present
+unchanged
 
 /* v3 */
-patch not present
+unchanged
 
 /* v4 */
-patch not present
+- Serge Hallyn <serge@hallyn.com>:
+  - Use "mnt_userns" to refer to a vfsmount's userns everywhere to make
+    terminology consistent.
 
 /* v5 */
-patch introduced
 base-commit: 7c53f6b671f4aba70ff15e1b05148b10d58c2837
+
+- Christoph Hellwig <hch@lst.de>:
+  - Use new file_mnt_user_ns() helper.
+
+/* v6 */
+base-commit: 19c329f6808995b142b3966301f217c831e7cf31
+
+- Christoph Hellwig <hch@lst.de>:
+  - Remove local mnt_userns variable in favor of calling
+    file_mnt_user_ns() directly.
 ---
- fs/namei.c         | 6 ++++++
- include/linux/fs.h | 1 +
- 2 files changed, 7 insertions(+)
-
+ fs/coredump.c      | 10 +++++++---
+ fs/inode.c         |  7 ++++---
+ fs/namei.c         |  2 +-
+ fs/open.c          | 16 +++++++++-------
+ include/linux/fs.h |  4 ++--
+ 5 files changed, 23 insertions(+), 16 deletions(-)
+
+diff --git a/fs/coredump.c b/fs/coredump.c
+index a2f6ecc8e345..ae778937a1ff 100644
+--- a/fs/coredump.c
++++ b/fs/coredump.c
+@@ -703,6 +703,7 @@ void do_coredump(const kernel_siginfo_t *siginfo)
+ 			goto close_fail;
+ 		}
+ 	} else {
++		struct user_namespace *mnt_userns;
+ 		struct inode *inode;
+ 		int open_flags = O_CREAT | O_RDWR | O_NOFOLLOW |
+ 				 O_LARGEFILE | O_EXCL;
+@@ -780,13 +781,15 @@ void do_coredump(const kernel_siginfo_t *siginfo)
+ 		 * a process dumps core while its cwd is e.g. on a vfat
+ 		 * filesystem.
+ 		 */
+-		if (!uid_eq(inode->i_uid, current_fsuid()))
++		mnt_userns = file_mnt_user_ns(cprm.file);
++		if (!uid_eq(i_uid_into_mnt(mnt_userns, inode), current_fsuid()))
+ 			goto close_fail;
+ 		if ((inode->i_mode & 0677) != 0600)
+ 			goto close_fail;
+ 		if (!(cprm.file->f_mode & FMODE_CAN_WRITE))
+ 			goto close_fail;
+-		if (do_truncate(cprm.file->f_path.dentry, 0, 0, cprm.file))
++		if (do_truncate(mnt_userns, cprm.file->f_path.dentry,
++				0, 0, cprm.file))
+ 			goto close_fail;
+ 	}
+ 
+@@ -931,7 +934,8 @@ void dump_truncate(struct coredump_params *cprm)
+ 	if (file->f_op->llseek && file->f_op->llseek != no_llseek) {
+ 		offset = file->f_op->llseek(file, 0, SEEK_CUR);
+ 		if (i_size_read(file->f_mapping->host) < offset)
+-			do_truncate(file->f_path.dentry, offset, 0, file);
++			do_truncate(file_mnt_user_ns(file), file->f_path.dentry,
++				    offset, 0, file);
+ 	}
+ }
+ EXPORT_SYMBOL(dump_truncate);
+diff --git a/fs/inode.c b/fs/inode.c
+index 46116ef44c9f..08151968c9ef 100644
+--- a/fs/inode.c
++++ b/fs/inode.c
+@@ -1903,7 +1903,8 @@ int dentry_needs_remove_privs(struct dentry *dentry)
+ 	return mask;
+ }
+ 
+-static int __remove_privs(struct dentry *dentry, int kill)
++static int __remove_privs(struct user_namespace *mnt_userns,
++			  struct dentry *dentry, int kill)
+ {
+ 	struct iattr newattrs;
+ 
+@@ -1912,7 +1913,7 @@ static int __remove_privs(struct dentry *dentry, int kill)
+ 	 * Note we call this on write, so notify_change will not
+ 	 * encounter any conflicting delegations:
+ 	 */
+-	return notify_change(&init_user_ns, dentry, &newattrs, NULL);
++	return notify_change(mnt_userns, dentry, &newattrs, NULL);
+ }
+ 
+ /*
+@@ -1939,7 +1940,7 @@ int file_remove_privs(struct file *file)
+ 	if (kill < 0)
+ 		return kill;
+ 	if (kill)
+-		error = __remove_privs(dentry, kill);
++		error = __remove_privs(file_mnt_user_ns(file), dentry, kill);
+ 	if (!error)
+ 		inode_has_no_xattr(inode);
+ 
 diff --git a/fs/namei.c b/fs/namei.c
-index cd124e18cec5..d8dee449e92a 100644
+index 5c9f6f8e90c4..c8c083daf368 100644
 --- a/fs/namei.c
 +++ b/fs/namei.c
-@@ -259,6 +259,12 @@ void putname(struct filename *name)
- 		__putname(name);
- }
- 
-+struct user_namespace *file_user_ns(struct file *file)
-+{
-+	return mnt_user_ns(file->f_path.mnt);
-+}
-+EXPORT_SYMBOL(file_user_ns);
-+
- /**
-  * check_acl - perform ACL permission checking
-  * @mnt_userns:	user namespace of the mount the inode was found from
+@@ -3009,7 +3009,7 @@ static int handle_truncate(struct file *filp)
+ 	if (!error)
+ 		error = security_path_truncate(path);
+ 	if (!error) {
+-		error = do_truncate(path->dentry, 0,
++		error = do_truncate(&init_user_ns, path->dentry, 0,
+ 				    ATTR_MTIME|ATTR_CTIME|ATTR_OPEN,
+ 				    filp);
+ 	}
+diff --git a/fs/open.c b/fs/open.c
+index c3e4dc43dd8d..8b3f3eb652d0 100644
+--- a/fs/open.c
++++ b/fs/open.c
+@@ -35,8 +35,8 @@
+ 
+ #include "internal.h"
+ 
+-int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs,
+-	struct file *filp)
++int do_truncate(struct user_namespace *mnt_userns, struct dentry *dentry,
++		loff_t length, unsigned int time_attrs, struct file *filp)
+ {
+ 	int ret;
+ 	struct iattr newattrs;
+@@ -61,13 +61,14 @@ int do_truncate(struct dentry *dentry, loff_t length, unsigned int time_attrs,
+ 
+ 	inode_lock(dentry->d_inode);
+ 	/* Note any delegations or leases have already been broken: */
+-	ret = notify_change(&init_user_ns, dentry, &newattrs, NULL);
++	ret = notify_change(mnt_userns, dentry, &newattrs, NULL);
+ 	inode_unlock(dentry->d_inode);
+ 	return ret;
+ }
+ 
+ long vfs_truncate(const struct path *path, loff_t length)
+ {
++	struct user_namespace *mnt_userns;
+ 	struct inode *inode;
+ 	long error;
+ 
+@@ -83,7 +84,8 @@ long vfs_truncate(const struct path *path, loff_t length)
+ 	if (error)
+ 		goto out;
+ 
+-	error = inode_permission(&init_user_ns, inode, MAY_WRITE);
++	mnt_userns = mnt_user_ns(path->mnt);
++	error = inode_permission(mnt_userns, inode, MAY_WRITE);
+ 	if (error)
+ 		goto mnt_drop_write_and_out;
+ 
+@@ -107,7 +109,7 @@ long vfs_truncate(const struct path *path, loff_t length)
+ 	if (!error)
+ 		error = security_path_truncate(path);
+ 	if (!error)
+-		error = do_truncate(path->dentry, length, 0, NULL);
++		error = do_truncate(mnt_userns, path->dentry, length, 0, NULL);
+ 
+ put_write_and_out:
+ 	put_write_access(inode);
+@@ -186,13 +188,13 @@ long do_sys_ftruncate(unsigned int fd, loff_t length, int small)
+ 	/* Check IS_APPEND on real upper inode */
+ 	if (IS_APPEND(file_inode(f.file)))
+ 		goto out_putf;
+-
+ 	sb_start_write(inode->i_sb);
+ 	error = locks_verify_truncate(inode, f.file, length);
+ 	if (!error)
+ 		error = security_path_truncate(&f.file->f_path);
+ 	if (!error)
+-		error = do_truncate(dentry, length, ATTR_MTIME|ATTR_CTIME, f.file);
++		error = do_truncate(file_mnt_user_ns(f.file), dentry, length,
++				    ATTR_MTIME | ATTR_CTIME, f.file);
+ 	sb_end_write(inode->i_sb);
+ out_putf:
+ 	fdput(f);
 diff --git a/include/linux/fs.h b/include/linux/fs.h
-index ef993857682b..89e41a821bad 100644
+index 29d7b2fe7de4..f0601cca1930 100644
 --- a/include/linux/fs.h
 +++ b/include/linux/fs.h
-@@ -2585,6 +2585,7 @@ static inline struct file *file_clone_open(struct file *file)
- 	return dentry_open(&file->f_path, file->f_flags, file->f_cred);
- }
- extern int filp_close(struct file *, fl_owner_t id);
-+extern struct user_namespace *file_user_ns(struct file *file);
- 
- extern struct filename *getname_flags(const char __user *, int, int *);
- extern struct filename *getname(const char __user *);
+@@ -2593,8 +2593,8 @@ static inline struct user_namespace *file_mnt_user_ns(struct file *file)
+ 	return mnt_user_ns(file->f_path.mnt);
+ }
+ extern long vfs_truncate(const struct path *, loff_t);
+-extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,
+-		       struct file *filp);
++int do_truncate(struct user_namespace *, struct dentry *, loff_t start,
++		unsigned int time_attrs, struct file *filp);
+ extern int vfs_fallocate(struct file *file, int mode, loff_t offset,
+ 			loff_t len);
+ extern long do_sys_open(int dfd, const char __user *filename, int flags,
 -- 
 2.30.0
 
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help