[RFC,v4,09/17] fs: Split off remove_needs_file_privs() __remove_file_privs()

Commit Message

Stefan Roesch May 20, 2022, 6:36 p.m. UTC

This splits off the function remove_needs_file_privs() from the function
__remove_file_privs() from the function file_remove_privs().

No intended functional changes in this patch.

Signed-off-by: Stefan Roesch <[e-mail]>
---
 fs/inode.c | 75 +++++++++++++++++++++++++++++++++++++++---------------
 1 file changed, 55 insertions(+), 20 deletions(-)

Comments

Christoph Hellwig May 22, 2022, 7:32 a.m. UTC | #1

So why don't we just define a __file_remove_privs that gets the iocb
flags passed an an extra argument, and then make file_remove_privs pass
0 flags, and maybe also add a kiocb_remove_privs wrapper for callers
that have the kiocb.  Same for the timestamp update, btw.  That seems
less churn and less code overall.

Stefan Roesch May 25, 2022, 9:34 p.m. UTC | #2

On 5/22/22 12:32 AM, Christoph Hellwig wrote:
> So why don't we just define a __file_remove_privs that gets the iocb
> flags passed an an extra argument, and then make file_remove_privs pass
> 0 flags, and maybe also add a kiocb_remove_privs wrapper for callers
> that have the kiocb.  Same for the timestamp update, btw.  That seems
> less churn and less code overall.
 I introduced a __file_remove_privs() function with an additional parameter
that takes the iocb_flags.

Patch

diff --git a/fs/inode.c b/fs/inode.c
index 9d9b422504d1..1bb8b7db836f 100644
--- a/fs/inode.c
+++ b/fs/inode.c
@@ -2010,17 +2010,8 @@  static int __remove_privs(struct user_namespace *mnt_userns,
 	return notify_change(mnt_userns, dentry, &newattrs, NULL);
 }
 
-/*
- * Remove special file priviledges (suid, capabilities) when file is written
- * to or truncated.
- */
-int file_remove_privs(struct file *file)
+static int file_needs_remove_privs(struct inode *inode, struct dentry *dentry)
 {
-	struct dentry *dentry = file_dentry(file);
-	struct inode *inode = file_inode(file);
-	int kill;
-	int error = 0;
-
 	/*
 	 * Fast path for nothing security related.
 	 * As well for non-regular files, e.g. blkdev inodes.
@@ -2030,16 +2021,42 @@  int file_remove_privs(struct file *file)
 	if (IS_NOSEC(inode) || !S_ISREG(inode->i_mode))
 		return 0;
 
-	kill = dentry_needs_remove_privs(dentry);
-	if (kill < 0)
-		return kill;
-	if (kill)
-		error = __remove_privs(file_mnt_user_ns(file), dentry, kill);
+	return dentry_needs_remove_privs(dentry);
+}
+
+static int __file_remove_privs(struct file *file, struct inode *inode,
+			struct dentry *dentry, int kill)
+{
+	int error = 0;
+
+	error = __remove_privs(file_mnt_user_ns(file), dentry, kill);
 	if (!error)
 		inode_has_no_xattr(inode);
 
 	return error;
 }
+
+/**
+ * file_remove_privs - remove special file privileges (suid, capabilities)
+ * @file: file to remove privileges from
+ *
+ * When file is modified by a write or truncation ensure that special
+ * file privileges are removed.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
+int file_remove_privs(struct file *file)
+{
+	struct dentry *dentry = file_dentry(file);
+	struct inode *inode = file_inode(file);
+	int kill;
+
+	kill = file_needs_remove_privs(inode, dentry);
+	if (kill <= 0)
+		return kill;
+
+	return __file_remove_privs(file, inode, dentry, kill);
+}
 EXPORT_SYMBOL(file_remove_privs);
 
 /**
@@ -2090,18 +2107,36 @@  int file_update_time(struct file *file)
 }
 EXPORT_SYMBOL(file_update_time);
 
-/* Caller must hold the file's inode lock */
+/**
+ * file_modified - handle mandated vfs changes when modifying a file
+ * @file: file that was modified
+ *
+ * When file has been modified ensure that special
+ * file privileges are removed and time settings are updated.
+ *
+ * Context: Caller must hold the file's inode lock.
+ *
+ * Return: 0 on success, negative errno on failure.
+ */
 int file_modified(struct file *file)
 {
-	int err;
+	int ret;
+	struct dentry *dentry = file_dentry(file);
+	struct inode *inode = file_inode(file);
 
 	/*
 	 * Clear the security bits if the process is not being run by root.
 	 * This keeps people from modifying setuid and setgid binaries.
 	 */
-	err = file_remove_privs(file);
-	if (err)
-		return err;
+	ret = file_needs_remove_privs(inode, dentry);
+	if (ret < 0)
+		return ret;
+
+	if (ret > 0) {
+		ret = __file_remove_privs(file, inode, dentry, ret);
+		if (ret)
+			return ret;
+	}
 
 	if (unlikely(file->f_mode & FMODE_NOCMTIME))
 		return 0;