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@xxxxxxxxxxxxxxx # 4.4+
> Signed-off-by: David Sterba <dsterba@xxxxxxxx>
> ---
> 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
>
