[v2] btrfs: add missing check for nocow and compression inode flags
diff mbox series

Message ID 20200713103349.22448-1-dsterba@suse.com
State New
Headers show
Series
  • [v2] btrfs: add missing check for nocow and compression inode flags
Related show

Commit Message

David Sterba July 13, 2020, 10:33 a.m. UTC
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

Inode attributes take precedence over mount options and are an
independent setting.

Nocompress would be a no-op with nodatacow, but we don't want to mix
any compression-related options with nodatacow.

CC: stable@vger.kernel.org # 4.4+
Signed-off-by: David Sterba <dsterba@suse.com>
---

v2:
- update chanelog: mount options vs attributes, nocompress vs nodatacow

 fs/btrfs/ioctl.c | 30 ++++++++++++++++++++++--------
 1 file changed, 22 insertions(+), 8 deletions(-)

Comments

Sasha Levin July 17, 2020, 5:08 p.m. UTC | #1
Hi

[This is an automated email]

This commit has been processed because it contains a -stable tag.
The stable tag indicates that it's relevant for the following trees: 4.4+

The bot has tested the following trees: v5.7.8, v5.4.51, v4.19.132, v4.14.188, v4.9.230, v4.4.230.

v5.7.8: Build OK!
v5.4.51: Build OK!
v4.19.132: Failed to apply! Possible dependencies:
    04e6863b19c72 ("btrfs: split btrfs_setxattr calls regarding transaction")
    262c96a3c3670 ("btrfs: refactor btrfs_set_prop and add btrfs_set_prop_trans")
    7715da84f74d5 ("btrfs: merge _btrfs_set_prop helpers")
    8b4d1efc9e6c3 ("btrfs: prop: open code btrfs_set_prop in inherit_prop")
    cac237ae095f6 ("btrfs: rename btrfs_setxattr to btrfs_setxattr_trans")
    d2b8fcfe43155 ("btrfs: modify local copy of btrfs_inode flags")
    f22125e5d8ae1 ("btrfs: refactor btrfs_set_props to validate externally")
    ff9fef559babe ("btrfs: start transaction in btrfs_ioctl_setflags()")

v4.14.188: Failed to apply! Possible dependencies:
    04e6863b19c72 ("btrfs: split btrfs_setxattr calls regarding transaction")
    1905a0f7c7de3 ("btrfs: rename btrfs_mask_flags to reflect which flags it touches")
    262c96a3c3670 ("btrfs: refactor btrfs_set_prop and add btrfs_set_prop_trans")
    38e82de8ccd18 ("btrfs: user proper type for btrfs_mask_flags flags")
    5ba76abfb2336 ("btrfs: rename check_flags to reflect which flags it touches")
    5c57b8b6a4966 ("btrfs: unify naming of flags variables for SETFLAGS and XFLAGS")
    7715da84f74d5 ("btrfs: merge _btrfs_set_prop helpers")
    7852781d94b30 ("btrfs: drop underscores from exported xattr functions")
    7b6a221e5b21f ("btrfs: rename btrfs_update_iflags to reflect which flags it touches")
    8b4d1efc9e6c3 ("btrfs: prop: open code btrfs_set_prop in inherit_prop")
    93370509c24cc ("btrfs: SETFLAGS ioctl: use helper for compression type conversion")
    a157d4fd81dc7 ("btrfs: rename btrfs_flags_to_ioctl to reflect which flags it touches")
    ab0d09361662b ("btrfs: drop extern from function declarations")
    cac237ae095f6 ("btrfs: rename btrfs_setxattr to btrfs_setxattr_trans")
    d2b8fcfe43155 ("btrfs: modify local copy of btrfs_inode flags")
    f22125e5d8ae1 ("btrfs: refactor btrfs_set_props to validate externally")
    ff9fef559babe ("btrfs: start transaction in btrfs_ioctl_setflags()")

v4.9.230: Failed to apply! Possible dependencies:
    0b246afa62b0c ("btrfs: root->fs_info cleanup, add fs_info convenience variables")
    1905a0f7c7de3 ("btrfs: rename btrfs_mask_flags to reflect which flags it touches")
    38e82de8ccd18 ("btrfs: user proper type for btrfs_mask_flags flags")
    5ba76abfb2336 ("btrfs: rename check_flags to reflect which flags it touches")
    5c57b8b6a4966 ("btrfs: unify naming of flags variables for SETFLAGS and XFLAGS")
    62d1f9fe97dd2 ("btrfs: remove trivial helper btrfs_find_tree_block")
    a157d4fd81dc7 ("btrfs: rename btrfs_flags_to_ioctl to reflect which flags it touches")
    cf8cddd38bab3 ("btrfs: don't abuse REQ_OP_* flags for btrfs_map_block")
    da17066c40472 ("btrfs: pull node/sector/stripe sizes out of root and into fs_info")
    de143792253e2 ("btrfs: struct btrfsic_state->root should be an fs_info")
    fb456252d3d9c ("btrfs: root->fs_info cleanup, use fs_info->dev_root everywhere")
    ff9fef559babe ("btrfs: start transaction in btrfs_ioctl_setflags()")

v4.4.230: Failed to apply! Possible dependencies:
    0132761017e01 ("btrfs: fix string and comment grammatical issues and typos")
    09cbfeaf1a5a6 ("mm, fs: get rid of PAGE_CACHE_* and page_cache_{get,release} macros")
    0b246afa62b0c ("btrfs: root->fs_info cleanup, add fs_info convenience variables")
    0e749e54244ee ("dax: increase granularity of dax_clear_blocks() operations")
    1905a0f7c7de3 ("btrfs: rename btrfs_mask_flags to reflect which flags it touches")
    38e82de8ccd18 ("btrfs: user proper type for btrfs_mask_flags flags")
    4420cfd3f51cf ("staging: lustre: format properly all comment blocks for LNet core")
    52db400fcd502 ("pmem, dax: clean up clear_pmem()")
    5ba76abfb2336 ("btrfs: rename check_flags to reflect which flags it touches")
    5c57b8b6a4966 ("btrfs: unify naming of flags variables for SETFLAGS and XFLAGS")
    5fd88337d209d ("staging: lustre: fix all conditional comparison to zero in LNet layer")
    a157d4fd81dc7 ("btrfs: rename btrfs_flags_to_ioctl to reflect which flags it touches")
    b2e0d1625e193 ("dax: fix lifetime of in-kernel dax mappings with dax_map_atomic()")
    bb7ab3b92e46d ("btrfs: Fix misspellings in comments.")
    cf8cddd38bab3 ("btrfs: don't abuse REQ_OP_* flags for btrfs_map_block")
    d1a5f2b4d8a12 ("block: use DAX for partition table reads")
    de143792253e2 ("btrfs: struct btrfsic_state->root should be an fs_info")
    e10624f8c0971 ("pmem: fail io-requests to known bad blocks")
    ff9fef559babe ("btrfs: start transaction in btrfs_ioctl_setflags()")


NOTE: The patch will not be queued to stable trees until it is upstream.

How should we proceed with this patch?

Patch
diff mbox series

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)))
+		return -EINVAL;
+	if ((flags & FS_NOCOW_FL) && (old_flags & (FS_COMPR_FL | FS_NOCOMP_FL)))
+		return -EINVAL;
+
 	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