[SRU Kinetic, Lunar, OEM-6.1 2/3] Revert "UBUNTU: SAUCE: overlayfs: Skip permission checking for trusted.overlayfs.* xattrs"

[Thadeu Lima de Souza Cascardo]

This reverts commit 2c7ab1423973cfb50e1226e6608a1d454e702c90.

Commit "UBUNTU: SAUCE: overlayfs: Skip permission checking for
trusted.overlayfs.* xattrs" replaced the VFS calls to change xattrs to
their _noperm equivalents.

However, since upstream commit c914c0e27eb0 ("ovl: use wrappers to all
vfs_*xattr() calls"), overlayfs started using the changed wrapper function
to set any extended attributes.

This means that overlayfs skips checking permissions for any extended
attribute changes, not only trusted.overlayfs.* xattrs, as was intended by
the sauce commit above.

Fixes: c914c0e27eb0 ("ovl: use wrappers to all vfs_*xattr() calls")
CVE-2023-2640
CVE-2023-32629
Signed-off-by: Thadeu Lima de Souza Cascardo <cascardo at canonical.com>
---
 fs/overlayfs/overlayfs.h | 16 +++-------------
 fs/xattr.c               | 36 ++++++------------------------------
 include/linux/xattr.h    |  1 -
 3 files changed, 9 insertions(+), 44 deletions(-)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index e523d600da4e..1df7f850ff3b 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -251,12 +251,8 @@ static inline int ovl_do_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
 				  const char *name, const void *value,
 				  size_t size, int flags)
 {
-	struct inode *inode = dentry->d_inode;
-	int err;
-
-	inode_lock(inode);
-	err = __vfs_setxattr_noperm(&init_user_ns, dentry, name, value, size, flags);
-	inode_unlock(inode);
+	int err = vfs_setxattr(ovl_upper_mnt_userns(ofs), dentry, name,
+			       value, size, flags);
 
 	pr_debug("setxattr(%pd2, \"%s\", \"%*pE\", %zu, %d) = %i\n",
 		 dentry, name, min((int)size, 48), value, size, flags, err);
@@ -273,13 +269,7 @@ static inline int ovl_setxattr(struct ovl_fs *ofs, struct dentry *dentry,
 static inline int ovl_do_removexattr(struct ovl_fs *ofs, struct dentry *dentry,
 				     const char *name)
 {
-	struct inode *inode = dentry->d_inode;
-	int err;
-
-	inode_lock(inode);
-	err = __vfs_removexattr_noperm(&init_user_ns, dentry, name);
-	inode_unlock(inode);
-
+	int err = vfs_removexattr(ovl_upper_mnt_userns(ofs), dentry, name);
 	pr_debug("removexattr(%pd2, \"%s\") = %i\n", dentry, name, err);
 	return err;
 }
diff --git a/fs/xattr.c b/fs/xattr.c
index 76dc91ec7ae8..adab9a70b536 100644
--- a/fs/xattr.c
+++ b/fs/xattr.c
@@ -259,7 +259,6 @@ int __vfs_setxattr_noperm(struct user_namespace *mnt_userns,
 
 	return error;
 }
-EXPORT_SYMBOL_GPL(__vfs_setxattr_noperm);
 
 /**
  * __vfs_setxattr_locked - set an extended attribute while holding the inode
@@ -500,34 +499,6 @@ __vfs_removexattr(struct user_namespace *mnt_userns, struct dentry *dentry,
 }
 EXPORT_SYMBOL(__vfs_removexattr);
 
-/**
- *  __vfs_removexattr_noperm - perform removexattr operation without
- *  performing permission checks.
- *
- *  @dentry - object to perform setxattr on
- *  @name - xattr name to set
- *
- *  returns the result of the internal setxattr or setsecurity operations.
- *
- *  This function requires the caller to lock the inode's i_mutex before it
- *  is executed. It also assumes that the caller will make the appropriate
- *  permission checks.
- */
-int
-__vfs_removexattr_noperm(struct user_namespace *mnt_userns,
-			 struct dentry *dentry, const char *name)
-{
-	int error;
-
-	error =__vfs_removexattr(mnt_userns, dentry, name);
-	if (!error) {
-		fsnotify_xattr(dentry);
-		evm_inode_post_removexattr(dentry, name);
-	}
-	return error;
-}
-EXPORT_SYMBOL_GPL(__vfs_removexattr_noperm);
-
 /**
  * __vfs_removexattr_locked - set an extended attribute while holding the inode
  * lock
@@ -558,7 +529,12 @@ __vfs_removexattr_locked(struct user_namespace *mnt_userns,
 	if (error)
 		goto out;
 
-	error = __vfs_removexattr_noperm(mnt_userns, dentry, name);
+	error = __vfs_removexattr(mnt_userns, dentry, name);
+
+	if (!error) {
+		fsnotify_xattr(dentry);
+		evm_inode_post_removexattr(dentry, name);
+	}
 
 out:
 	return error;
diff --git a/include/linux/xattr.h b/include/linux/xattr.h
index 149b148625da..2e7dd44926e4 100644
--- a/include/linux/xattr.h
+++ b/include/linux/xattr.h
@@ -69,7 +69,6 @@ int __vfs_setxattr_locked(struct user_namespace *, struct dentry *,
 int vfs_setxattr(struct user_namespace *, struct dentry *, const char *,
 		 const void *, size_t, int);
 int __vfs_removexattr(struct user_namespace *, struct dentry *, const char *);
-int __vfs_removexattr_noperm(struct user_namespace *, struct dentry *, const char *);
 int __vfs_removexattr_locked(struct user_namespace *, struct dentry *,
 			     const char *, struct inode **);
 int vfs_removexattr(struct user_namespace *, struct dentry *, const char *);
-- 
2.34.1