From e4a3d39e87313954cb3e8214e28ee0ba50bee874 Mon Sep 17 00:00:00 2001 From: Neal Gompa Date: Jul 24 2020 13:04:17 +0000 Subject: Backport fix for converting 64-bit ext4 filesystems (#1851674) --- diff --git a/0001-btrfs-progs-convert-prevent-32bit-overflow-for-cctx-.patch b/0001-btrfs-progs-convert-prevent-32bit-overflow-for-cctx-.patch new file mode 100644 index 0000000..6fc0f3f --- /dev/null +++ b/0001-btrfs-progs-convert-prevent-32bit-overflow-for-cctx-.patch @@ -0,0 +1,108 @@ +From fe8715e1337a1bad5c49e165ab77c033c334efbc Mon Sep 17 00:00:00 2001 +From: Qu Wenruo +Date: Mon, 20 Jul 2020 20:51:08 +0800 +Subject: [PATCH 1/2] btrfs-progs: convert: prevent 32bit overflow for + cctx->total_bytes + +[BUG] +When convert is called on a 64GiB ext4 fs, it fails like this: + + $ btrfs-convert /dev/loop0p1 + create btrfs filesystem: + blocksize: 4096 + nodesize: 16384 + features: extref, skinny-metadata (default) + checksum: crc32c + creating ext2 image file + ERROR: missing data block for bytenr 1048576 + ERROR: failed to create ext2_saved/image: -2 + WARNING: an error occurred during conversion, filesystem is partially created but not finalized and not mountable + +Btrfs-convert also corrupts the source fs: + + $ LANG=C e2fsck /dev/loop0p1 -f + e2fsck 1.45.6 (20-Mar-2020) + Resize inode not valid. Recreate? yes + Pass 1: Checking inodes, blocks, and sizes + Deleted inode 3681 has zero dtime. Fix? yes + Inodes that were part of a corrupted orphan linked list found. Fix? yes + Inode 3744 was part of the orphaned inode list. FIXED. + Deleted inode 3745 has zero dtime. Fix? yes + Inode 3747 has INLINE_DATA_FL flag on filesystem without inline data support. + Clear? yes + ... + +[CAUSE] +After some debugging, the first strange behavior is, the value of +cctx->total_bytes is 0 in ext2_open_fs(). + +It turns out that, the value assign for cctx->total_bytes could lead to +bit overflow for the unsigned int value. + +And that 0 cctx->total_bytes leads to various problems for later free +space calculation. +For example, in calculate_available_space(), we use cctx->total_bytes to +ensure we won't create a data chunk beyond device end: + + cue_len = min(cctx->total_bytes - cur_off, cur_len); + +If that cur_offset is also 0, we will create a cache_extent with 0 size, +which could cause a lot of problems for cache tree search. + +[FIX] +Do manual casting for the multiply operation, so we could got a real u64 +result. The fix will be applied to all supported fses (ext* and +reiserfs). + +Reported-by: Christian Zangl +Reviewed-by: Nikolay Borisov +Signed-off-by: Qu Wenruo +Signed-off-by: David Sterba +(cherry picked from commit 670a19828ad40004d05ad70cdd45d58008d3fb51) +--- + convert/main.c | 1 + + convert/source-ext2.c | 2 +- + convert/source-reiserfs.c | 2 +- + 3 files changed, 3 insertions(+), 2 deletions(-) + +diff --git a/convert/main.c b/convert/main.c +index 7709e9a6..df6a2ae3 100644 +--- a/convert/main.c ++++ b/convert/main.c +@@ -1136,6 +1136,7 @@ static int do_convert(const char *devname, u32 convert_flags, u32 nodesize, + if (ret) + goto fail; + ++ ASSERT(cctx.total_bytes); + blocksize = cctx.blocksize; + total_bytes = (u64)blocksize * (u64)cctx.block_count; + if (blocksize < 4096) { +diff --git a/convert/source-ext2.c b/convert/source-ext2.c +index f11ef651..d73684ef 100644 +--- a/convert/source-ext2.c ++++ b/convert/source-ext2.c +@@ -87,7 +87,7 @@ static int ext2_open_fs(struct btrfs_convert_context *cctx, const char *name) + cctx->fs_data = ext2_fs; + cctx->blocksize = ext2_fs->blocksize; + cctx->block_count = ext2_fs->super->s_blocks_count; +- cctx->total_bytes = ext2_fs->blocksize * ext2_fs->super->s_blocks_count; ++ cctx->total_bytes = (u64)ext2_fs->super->s_blocks_count * ext2_fs->blocksize; + cctx->volume_name = strndup((char *)ext2_fs->super->s_volume_name, 16); + cctx->first_data_block = ext2_fs->super->s_first_data_block; + cctx->inodes_count = ext2_fs->super->s_inodes_count; +diff --git a/convert/source-reiserfs.c b/convert/source-reiserfs.c +index 9fd6b9ab..3b4cb5ad 100644 +--- a/convert/source-reiserfs.c ++++ b/convert/source-reiserfs.c +@@ -82,7 +82,7 @@ static int reiserfs_open_fs(struct btrfs_convert_context *cxt, const char *name) + cxt->fs_data = fs; + cxt->blocksize = fs->fs_blocksize; + cxt->block_count = get_sb_block_count(fs->fs_ondisk_sb); +- cxt->total_bytes = cxt->blocksize * cxt->block_count; ++ cxt->total_bytes = (u64)cxt->block_count * cxt->blocksize; + cxt->volume_name = strndup(fs->fs_ondisk_sb->s_label, 16); + cxt->first_data_block = 0; + cxt->inodes_count = reiserfs_count_objectids(fs); +-- +2.26.2 + diff --git a/0002-btrfs-progs-tests-add-convert-test-case-for-multiply.patch b/0002-btrfs-progs-tests-add-convert-test-case-for-multiply.patch new file mode 100644 index 0000000..ab5d642 --- /dev/null +++ b/0002-btrfs-progs-tests-add-convert-test-case-for-multiply.patch @@ -0,0 +1,49 @@ +From 11c4a7b38a6aeef51b075d70e5aca36c42398842 Mon Sep 17 00:00:00 2001 +From: Qu Wenruo +Date: Mon, 20 Jul 2020 20:51:09 +0800 +Subject: [PATCH 2/2] btrfs-progs: tests: add convert test case for multiply + overflow + +The new test case will test whether btrfs-convert can handle 64G ext* +fs. + +This exercise the cctx->total_bytes calculation where multiplying 4K +(unsigned int) and 16777216 (u32) could lead to bit overflow for +unsigned int and got 0, and screw up later free space calculation. + +Signed-off-by: Qu Wenruo +Signed-off-by: David Sterba +(cherry picked from commit 61f41474469b0b0fba9ec1978dfa91d9cbc6c914) +--- + .../018-fs-size-overflow/test.sh | 19 +++++++++++++++++++ + 1 file changed, 19 insertions(+) + create mode 100755 tests/convert-tests/018-fs-size-overflow/test.sh + +diff --git a/tests/convert-tests/018-fs-size-overflow/test.sh b/tests/convert-tests/018-fs-size-overflow/test.sh +new file mode 100755 +index 00000000..d819f695 +--- /dev/null ++++ b/tests/convert-tests/018-fs-size-overflow/test.sh +@@ -0,0 +1,19 @@ ++#!/bin/bash ++# Check if btrfs-convert can handle an ext4 fs whose size is 64G. ++# That fs size could trigger a multiply overflow and screw up free space ++# calculation ++ ++source "$TEST_TOP/common" ++source "$TEST_TOP/common.convert" ++ ++setup_root_helper ++prepare_test_dev 64g ++check_prereq btrfs-convert ++check_global_prereq mke2fs ++ ++convert_test_prep_fs ext4 mke2fs -t ext4 -b 4096 ++run_check_umount_test_dev ++ ++# Unpatched btrfs-convert would fail half way due to corrupted free space ++# cache tree ++convert_test_do_convert +-- +2.26.2 + diff --git a/btrfs-progs.spec b/btrfs-progs.spec index 761e199..e6bf71a 100644 --- a/btrfs-progs.spec +++ b/btrfs-progs.spec @@ -3,7 +3,7 @@ Name: btrfs-progs Version: 5.7 -Release: 3%{?dist} +Release: 4%{?dist} Summary: Userspace programs for btrfs License: GPLv2 @@ -11,8 +11,12 @@ URL: https://btrfs.wiki.kernel.org/index.php/Main_Page Source0: https://www.kernel.org/pub/linux/kernel/people/kdave/%{name}/%{name}-v%{version_no_tilde}.tar.xz # Backports from upstream +## Do not use raid0 by default for mkfs multi-disk (#1855174) Patch0001: 0001-btrfs-progs-mkfs-clean-up-default-profile-settings.patch Patch0002: 0002-btrfs-progs-mkfs-switch-to-single-as-default-profile.patch +## Fix convert for 64-bit ext4 filesystems (#1851674) +Patch0003: 0001-btrfs-progs-convert-prevent-32bit-overflow-for-cctx-.patch +Patch0004: 0002-btrfs-progs-tests-add-convert-test-case-for-multiply.patch BuildRequires: gcc, autoconf, automake BuildRequires: e2fsprogs-devel, libuuid-devel, zlib-devel, libzstd-devel @@ -146,6 +150,9 @@ popd %endif %changelog +* Fri Jul 24 2020 Neal Gompa - 5.7-4 +- Backport fix for converting 64-bit ext4 filesystems (#1851674) + * Tue Jul 21 2020 Neal Gompa - 5.7-3 - Backport fix to not use raid0 by default for mkfs multi-disk (#1855174)