On 10.07.20 г. 13:05 ч., David Sterba wrote:
User Forza reported on IRC that some invalid combinations of file attributes are accepted by chattr.
The NODATACOW and compression file flags/attributes are mutually exclusive, but they could be set by 'chattr +c +C' on an empty file. The nodatacow will be in effect because it's checked first in btrfs_run_delalloc_range.
Extend the flag validation to catch the following cases:
- input flags are conflicting
- old and new flags are conflicting
- initialize the local variable with inode flags after inode ls locked
CC: stable@vger.kernel.org # 4.4+ Signed-off-by: David Sterba dsterba@suse.com
fs/btrfs/ioctl.c | 30 ++++++++++++++++++++++-------- 1 file changed, 22 insertions(+), 8 deletions(-)
diff --git a/fs/btrfs/ioctl.c b/fs/btrfs/ioctl.c index 3a566cf71fc6..0c13bb38425b 100644 --- a/fs/btrfs/ioctl.c +++ b/fs/btrfs/ioctl.c @@ -164,8 +164,11 @@ static int btrfs_ioctl_getflags(struct file *file, void __user *arg) return 0; } -/* Check if @flags are a supported and valid set of FS_*_FL flags */ -static int check_fsflags(unsigned int flags) +/*
- Check if @flags are a supported and valid set of FS_*_FL flags and that
- the old and new flags are not conflicting
- */
+static int check_fsflags(unsigned int old_flags, unsigned int flags) { if (flags & ~(FS_IMMUTABLE_FL | FS_APPEND_FL | \ FS_NOATIME_FL | FS_NODUMP_FL | \ @@ -174,9 +177,19 @@ static int check_fsflags(unsigned int flags) FS_NOCOW_FL)) return -EOPNOTSUPP;
- /* COMPR and NOCOMP on new/old are valid */ if ((flags & FS_NOCOMP_FL) && (flags & FS_COMPR_FL)) return -EINVAL;
- if ((flags & FS_COMPR_FL) && (flags & FS_NOCOW_FL))
return -EINVAL;
- /* NOCOW and compression options are mutually exclusive */
- if ((old_flags & FS_NOCOW_FL) && (flags & (FS_COMPR_FL | FS_NOCOMP_FL)))
Why is NOCOW and setting NOCOMP (which would really be a NOOP) an invalid combination?
return -EINVAL;
- if ((flags & FS_NOCOW_FL) && (old_flags & (FS_COMPR_FL | FS_NOCOMP_FL)))
return -EINVAL;
Same thing here, just inverted?
- return 0;
} @@ -190,7 +203,7 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) unsigned int fsflags, old_fsflags; int ret; const char *comp = NULL;
- u32 binode_flags = binode->flags;
- u32 binode_flags;
if (!inode_owner_or_capable(inode)) return -EPERM; @@ -201,22 +214,23 @@ static int btrfs_ioctl_setflags(struct file *file, void __user *arg) if (copy_from_user(&fsflags, arg, sizeof(fsflags))) return -EFAULT;
- ret = check_fsflags(fsflags);
- if (ret)
return ret;
- ret = mnt_want_write_file(file); if (ret) return ret;
inode_lock(inode);
- fsflags = btrfs_mask_fsflags_for_type(inode, fsflags); old_fsflags = btrfs_inode_flags_to_fsflags(binode->flags);
- ret = vfs_ioc_setflags_prepare(inode, old_fsflags, fsflags); if (ret) goto out_unlock;
- ret = check_fsflags(old_fsflags, fsflags);
- if (ret)
goto out_unlock;
- binode_flags = binode->flags; if (fsflags & FS_SYNC_FL) binode_flags |= BTRFS_INODE_SYNC; else