@@ -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;
}
@@ -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;
@@ -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 *);
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@canonical.com> --- fs/overlayfs/overlayfs.h | 16 +++------------- fs/xattr.c | 36 ++++++------------------------------ include/linux/xattr.h | 1 - 3 files changed, 9 insertions(+), 44 deletions(-)