]> git.proxmox.com Git - mirror_ubuntu-bionic-kernel.git/commitdiff
btrfs: inode: Don't compress if NODATASUM or NODATACOW set
authorQu Wenruo <wqu@suse.com>
Mon, 1 Jul 2019 05:12:46 +0000 (05:12 +0000)
committerKleber Sacilotto de Souza <kleber.souza@canonical.com>
Wed, 14 Aug 2019 09:18:49 +0000 (11:18 +0200)
BugLink: https://bugs.launchpad.net/bugs/1839036
commit 42c16da6d684391db83788eb680accd84f6c2083 upstream.

As btrfs(5) specified:

Note
If nodatacow or nodatasum are enabled, compression is disabled.

If NODATASUM or NODATACOW set, we should not compress the extent.

Normally NODATACOW is detected properly in run_delalloc_range() so
compression won't happen for NODATACOW.

However for NODATASUM we don't have any check, and it can cause
compressed extent without csum pretty easily, just by:
  mkfs.btrfs -f $dev
  mount $dev $mnt -o nodatasum
  touch $mnt/foobar
  mount -o remount,datasum,compress $mnt
  xfs_io -f -c "pwrite 0 128K" $mnt/foobar

And in fact, we have a bug report about corrupted compressed extent
without proper data checksum so even RAID1 can't recover the corruption.
(https://bugzilla.kernel.org/show_bug.cgi?id=199707)

Running compression without proper checksum could cause more damage when
corruption happens, as compressed data could make the whole extent
unreadable, so there is no need to allow compression for
NODATACSUM.

The fix will refactor the inode compression check into two parts:

- inode_can_compress()
  As the hard requirement, checked at btrfs_run_delalloc_range(), so no
  compression will happen for NODATASUM inode at all.

- inode_need_compress()
  As the soft requirement, checked at btrfs_run_delalloc_range() and
  compress_file_range().

Reported-by: James Harvey <jamespharvey20@gmail.com>
CC: stable@vger.kernel.org # 4.4+
Signed-off-by: Qu Wenruo <wqu@suse.com>
Reviewed-by: David Sterba <dsterba@suse.com>
Signed-off-by: David Sterba <dsterba@suse.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Kamal Mostafa <kamal@canonical.com>
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>
fs/btrfs/inode.c

index 087b82e583f53cfad872b07d9fded0dce5395cc7..e06e0f730d4ee2e7a3cb4f17a154c8be52b6a4c7 100644 (file)
@@ -405,10 +405,31 @@ static noinline int add_async_extent(struct async_cow *cow,
        return 0;
 }
 
+/*
+ * Check if the inode has flags compatible with compression
+ */
+static inline bool inode_can_compress(struct inode *inode)
+{
+       if (BTRFS_I(inode)->flags & BTRFS_INODE_NODATACOW ||
+           BTRFS_I(inode)->flags & BTRFS_INODE_NODATASUM)
+               return false;
+       return true;
+}
+
+/*
+ * Check if the inode needs to be submitted to compression, based on mount
+ * options, defragmentation, properties or heuristics.
+ */
 static inline int inode_need_compress(struct inode *inode, u64 start, u64 end)
 {
        struct btrfs_fs_info *fs_info = btrfs_sb(inode->i_sb);
 
+       if (!inode_can_compress(inode)) {
+               WARN(IS_ENABLED(CONFIG_BTRFS_DEBUG),
+                       KERN_ERR "BTRFS: unexpected compression for ino %llu\n",
+                       btrfs_ino(BTRFS_I(inode)));
+               return 0;
+       }
        /* force compress */
        if (btrfs_test_opt(fs_info, FORCE_COMPRESS))
                return 1;
@@ -1611,7 +1632,8 @@ static int run_delalloc_range(void *private_data, struct page *locked_page,
        } else if (BTRFS_I(inode)->flags & BTRFS_INODE_PREALLOC && !force_cow) {
                ret = run_delalloc_nocow(inode, locked_page, start, end,
                                         page_started, 0, nr_written);
-       } else if (!inode_need_compress(inode, start, end)) {
+       } else if (!inode_can_compress(inode) ||
+                  !inode_need_compress(inode, start, end)) {
                ret = cow_file_range(inode, locked_page, start, end, end,
                                      page_started, nr_written, 1, NULL);
        } else {