vfs-6.8-rc5.fixes

-----BEGIN PGP SIGNATURE-----
 
 iHUEABYKAB0WIQRAhzRXHqcMeLMyaSiRxhvAZXjcogUCZcoMdAAKCRCRxhvAZXjc
 ogy4AQDVp4huR6BBnRMhOCZbIsmkuHmq6ynpIZNTTAM0DdMn5AEAlJ03aEIaG9WS
 RQMdaYajeVpZfR/vIUg8UdVkHQxOEgw=
 =akNF
 -----END PGP SIGNATURE-----

Merge tag 'vfs-6.8-rc5.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs

Pull vfs fixes from Christian Brauner:

 - Fix performance regression introduced by moving the security
   permission hook out of do_clone_file_range() and into its caller
   vfs_clone_file_range().

   This causes the security hook to be called in situation were it
   wasn't called before as the fast permission checks were left in
   do_clone_file_range().

   Fix this by merging the two implementations back together and
   restoring the old ordering: fast permission checks first, expensive
   ones later.

 - Tweak mount_setattr() permission checking so that mount properties on
   the real rootfs can be changed.

   When we added mount_setattr() we added additional checks compared to
   legacy mount(2). If the mount had a parent then verify that the
   caller and the mount namespace the mount is attached to match and if
   not make sure that it's an anonymous mount.

   But the real rootfs falls into neither category. It is neither an
   anoymous mount because it is obviously attached to the initial mount
   namespace but it also obviously doesn't have a parent mount. So that
   means legacy mount(2) allows changing mount properties on the real
   rootfs but mount_setattr(2) blocks this. This causes regressions (See
   the commit for details).

   Fix this by relaxing the check. If the mount has a parent or if it
   isn't a detached mount, verify that the mount namespaces of the
   caller and the mount are the same. Technically, we could probably
   write this even simpler and check that the mount namespaces match if
   it isn't a detached mount. But the slightly longer check makes it
   clearer what conditions one needs to think about.

* tag 'vfs-6.8-rc5.fixes' of git://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs:
  fs: relax mount_setattr() permission checks
  remap_range: merge do_clone_file_range() into vfs_clone_file_range()
This commit is contained in:
Linus Torvalds 2024-02-12 07:15:45 -08:00
commit 716f4aaa7b
4 changed files with 23 additions and 36 deletions

View file

@ -4472,10 +4472,15 @@ static int do_mount_setattr(struct path *path, struct mount_kattr *kattr)
/*
* If this is an attached mount make sure it's located in the callers
* mount namespace. If it's not don't let the caller interact with it.
* If this is a detached mount make sure it has an anonymous mount
* namespace attached to it, i.e. we've created it via OPEN_TREE_CLONE.
*
* If this mount doesn't have a parent it's most often simply a
* detached mount with an anonymous mount namespace. IOW, something
* that's simply not attached yet. But there are apparently also users
* that do change mount properties on the rootfs itself. That obviously
* neither has a parent nor is it a detached mount so we cannot
* unconditionally check for detached mounts.
*/
if (!(mnt_has_parent(mnt) ? check_mnt(mnt) : is_anon_ns(mnt->mnt_ns)))
if ((mnt_has_parent(mnt) || !is_anon_ns(mnt->mnt_ns)) && !check_mnt(mnt))
goto out;
/*

View file

@ -265,20 +265,18 @@ static int ovl_copy_up_file(struct ovl_fs *ofs, struct dentry *dentry,
if (IS_ERR(old_file))
return PTR_ERR(old_file);
/* Try to use clone_file_range to clone up within the same fs */
cloned = vfs_clone_file_range(old_file, 0, new_file, 0, len, 0);
if (cloned == len)
goto out_fput;
/* Couldn't clone, so now we try to copy the data */
error = rw_verify_area(READ, old_file, &old_pos, len);
if (!error)
error = rw_verify_area(WRITE, new_file, &new_pos, len);
if (error)
goto out_fput;
/* Try to use clone_file_range to clone up within the same fs */
ovl_start_write(dentry);
cloned = do_clone_file_range(old_file, 0, new_file, 0, len, 0);
ovl_end_write(dentry);
if (cloned == len)
goto out_fput;
/* Couldn't clone, so now we try to copy the data */
/* Check if lower fs supports seek operation */
if (old_file->f_mode & FMODE_LSEEK)
skip_hole = true;

View file

@ -373,9 +373,9 @@ int generic_remap_file_range_prep(struct file *file_in, loff_t pos_in,
}
EXPORT_SYMBOL(generic_remap_file_range_prep);
loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
loff_t len, unsigned int remap_flags)
loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
loff_t len, unsigned int remap_flags)
{
loff_t ret;
@ -391,23 +391,6 @@ loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
if (!file_in->f_op->remap_file_range)
return -EOPNOTSUPP;
ret = file_in->f_op->remap_file_range(file_in, pos_in,
file_out, pos_out, len, remap_flags);
if (ret < 0)
return ret;
fsnotify_access(file_in);
fsnotify_modify(file_out);
return ret;
}
EXPORT_SYMBOL(do_clone_file_range);
loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
loff_t len, unsigned int remap_flags)
{
loff_t ret;
ret = remap_verify_area(file_in, pos_in, len, false);
if (ret)
return ret;
@ -417,10 +400,14 @@ loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
return ret;
file_start_write(file_out);
ret = do_clone_file_range(file_in, pos_in, file_out, pos_out, len,
remap_flags);
ret = file_in->f_op->remap_file_range(file_in, pos_in,
file_out, pos_out, len, remap_flags);
file_end_write(file_out);
if (ret < 0)
return ret;
fsnotify_access(file_in);
fsnotify_modify(file_out);
return ret;
}
EXPORT_SYMBOL(vfs_clone_file_range);

View file

@ -2101,9 +2101,6 @@ int __generic_remap_file_range_prep(struct file *file_in, loff_t pos_in,
int generic_remap_file_range_prep(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
loff_t *count, unsigned int remap_flags);
extern loff_t do_clone_file_range(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
loff_t len, unsigned int remap_flags);
extern loff_t vfs_clone_file_range(struct file *file_in, loff_t pos_in,
struct file *file_out, loff_t pos_out,
loff_t len, unsigned int remap_flags);