* [PATCH v3 0/3] btrfs: Add write time super block validation
@ 2018-05-11 5:35 Qu Wenruo
2018-05-11 5:35 ` [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration Qu Wenruo
` (3 more replies)
0 siblings, 4 replies; 15+ messages in thread
From: Qu Wenruo @ 2018-05-11 5:35 UTC (permalink / raw)
To: linux-btrfs
This patchset can be fetched from github:
https://github.com/adam900710/linux/tree/write_time_sb_check
We have 2 reports about corrupted btrfs super block, which has some garbage
in its super block, but otherwise it's completely fine and its csum even
matches.
This means we develop memory corruption during btrfs mount time.
It's not clear whether it's caused by btrfs or some other kernel module,
but at least let's do write time verification to catch such corruption
early.
Current design is to do 2 different checks at mount time and super write
time.
And for write time check, it only checks the template super block
(fs_info->super_to_commit) other than each super blocks to be written to
disk, mostly to avoid duplicated checks.
Changelog:
v2:
Rename btrfs_check_super_valid() to btrfs_validate_super() suggested
by Nikolay and David.
v3:
Add a new patch to move btrfs_check_super_valid() to avoid forward
declaration.
Refactor btrfs_check_super_valid() to provide better naming and
function reusablity.
Code style and comment update.
Use 2 different functions, btrfs_validate_mount_super() and
btrfs_validate_write_super(), for mount and write time super check.
Qu Wenruo (3):
btrfs: Move btrfs_check_super_valid() to avoid forward declaration
btrfs: Refactor btrfs_check_super_valid()
btrfs: Do super block verification before writing it to disk
fs/btrfs/disk-io.c | 361 ++++++++++++++++++++++++++-------------------
1 file changed, 210 insertions(+), 151 deletions(-)
--
2.17.0
^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-11 5:35 [PATCH v3 0/3] btrfs: Add write time super block validation Qu Wenruo
@ 2018-05-11 5:35 ` Qu Wenruo
2018-05-11 9:36 ` David Sterba
2018-05-24 16:24 ` Anand Jain
2018-05-11 5:35 ` [PATCH v3 2/3] btrfs: Refactor btrfs_check_super_valid() Qu Wenruo
` (2 subsequent siblings)
3 siblings, 2 replies; 15+ messages in thread
From: Qu Wenruo @ 2018-05-11 5:35 UTC (permalink / raw)
To: linux-btrfs
Just move btrfs_check_super_valid() before its single caller to avoid
forward declaration.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/disk-io.c | 299 ++++++++++++++++++++++-----------------------
1 file changed, 149 insertions(+), 150 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 932ed61d9554..13c5f90995aa 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -55,7 +55,6 @@
static const struct extent_io_ops btree_extent_io_ops;
static void end_workqueue_fn(struct btrfs_work *work);
static void free_fs_root(struct btrfs_root *root);
-static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info);
static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
struct btrfs_fs_info *fs_info);
@@ -2441,6 +2440,155 @@ static int btrfs_read_roots(struct btrfs_fs_info *fs_info)
return ret;
}
+static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
+{
+ struct btrfs_super_block *sb = fs_info->super_copy;
+ u64 nodesize = btrfs_super_nodesize(sb);
+ u64 sectorsize = btrfs_super_sectorsize(sb);
+ int ret = 0;
+
+ if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
+ btrfs_err(fs_info, "no valid FS found");
+ ret = -EINVAL;
+ }
+ if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
+ btrfs_err(fs_info, "unrecognized or unsupported super flag: %llu",
+ btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
+ ret = -EINVAL;
+ }
+ if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
+ btrfs_err(fs_info, "tree_root level too big: %d >= %d",
+ btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
+ ret = -EINVAL;
+ }
+ if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
+ btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
+ btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
+ ret = -EINVAL;
+ }
+ if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
+ btrfs_err(fs_info, "log_root level too big: %d >= %d",
+ btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
+ ret = -EINVAL;
+ }
+
+ /*
+ * Check sectorsize and nodesize first, other check will need it.
+ * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
+ */
+ if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
+ sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
+ btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
+ ret = -EINVAL;
+ }
+ /* Only PAGE SIZE is supported yet */
+ if (sectorsize != PAGE_SIZE) {
+ btrfs_err(fs_info,
+ "sectorsize %llu not supported yet, only support %lu",
+ sectorsize, PAGE_SIZE);
+ ret = -EINVAL;
+ }
+ if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
+ nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
+ btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
+ ret = -EINVAL;
+ }
+ if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
+ btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
+ le32_to_cpu(sb->__unused_leafsize), nodesize);
+ ret = -EINVAL;
+ }
+
+ /* Root alignment check */
+ if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
+ btrfs_warn(fs_info, "tree_root block unaligned: %llu",
+ btrfs_super_root(sb));
+ ret = -EINVAL;
+ }
+ if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
+ btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
+ btrfs_super_chunk_root(sb));
+ ret = -EINVAL;
+ }
+ if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
+ btrfs_warn(fs_info, "log_root block unaligned: %llu",
+ btrfs_super_log_root(sb));
+ ret = -EINVAL;
+ }
+
+ if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) != 0) {
+ btrfs_err(fs_info,
+ "dev_item UUID does not match fsid: %pU != %pU",
+ fs_info->fsid, sb->dev_item.fsid);
+ ret = -EINVAL;
+ }
+
+ /*
+ * Hint to catch really bogus numbers, bitflips or so, more exact checks are
+ * done later
+ */
+ if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
+ btrfs_err(fs_info, "bytes_used is too small %llu",
+ btrfs_super_bytes_used(sb));
+ ret = -EINVAL;
+ }
+ if (!is_power_of_2(btrfs_super_stripesize(sb))) {
+ btrfs_err(fs_info, "invalid stripesize %u",
+ btrfs_super_stripesize(sb));
+ ret = -EINVAL;
+ }
+ if (btrfs_super_num_devices(sb) > (1UL << 31))
+ btrfs_warn(fs_info, "suspicious number of devices: %llu",
+ btrfs_super_num_devices(sb));
+ if (btrfs_super_num_devices(sb) == 0) {
+ btrfs_err(fs_info, "number of devices is 0");
+ ret = -EINVAL;
+ }
+
+ if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
+ btrfs_err(fs_info, "super offset mismatch %llu != %u",
+ btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
+ ret = -EINVAL;
+ }
+
+ /*
+ * Obvious sys_chunk_array corruptions, it must hold at least one key
+ * and one chunk
+ */
+ if (btrfs_super_sys_array_size(sb) > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
+ btrfs_err(fs_info, "system chunk array too big %u > %u",
+ btrfs_super_sys_array_size(sb),
+ BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
+ ret = -EINVAL;
+ }
+ if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
+ + sizeof(struct btrfs_chunk)) {
+ btrfs_err(fs_info, "system chunk array too small %u < %zu",
+ btrfs_super_sys_array_size(sb),
+ sizeof(struct btrfs_disk_key)
+ + sizeof(struct btrfs_chunk));
+ ret = -EINVAL;
+ }
+
+ /*
+ * The generation is a global counter, we'll trust it more than the others
+ * but it's still possible that it's the one that's wrong.
+ */
+ if (btrfs_super_generation(sb) < btrfs_super_chunk_root_generation(sb))
+ btrfs_warn(fs_info,
+ "suspicious: generation < chunk_root_generation: %llu < %llu",
+ btrfs_super_generation(sb),
+ btrfs_super_chunk_root_generation(sb));
+ if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
+ && btrfs_super_cache_generation(sb) != (u64)-1)
+ btrfs_warn(fs_info,
+ "suspicious: generation < cache_generation: %llu < %llu",
+ btrfs_super_generation(sb),
+ btrfs_super_cache_generation(sb));
+
+ return ret;
+}
+
int open_ctree(struct super_block *sb,
struct btrfs_fs_devices *fs_devices,
char *options)
@@ -3972,155 +4120,6 @@ int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid, int level,
level, first_key);
}
-static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
-{
- struct btrfs_super_block *sb = fs_info->super_copy;
- u64 nodesize = btrfs_super_nodesize(sb);
- u64 sectorsize = btrfs_super_sectorsize(sb);
- int ret = 0;
-
- if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
- btrfs_err(fs_info, "no valid FS found");
- ret = -EINVAL;
- }
- if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
- btrfs_err(fs_info, "unrecognized or unsupported super flag: %llu",
- btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
- ret = -EINVAL;
- }
- if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
- btrfs_err(fs_info, "tree_root level too big: %d >= %d",
- btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
- ret = -EINVAL;
- }
- if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
- btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
- btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
- ret = -EINVAL;
- }
- if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
- btrfs_err(fs_info, "log_root level too big: %d >= %d",
- btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
- ret = -EINVAL;
- }
-
- /*
- * Check sectorsize and nodesize first, other check will need it.
- * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
- */
- if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
- sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
- btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
- ret = -EINVAL;
- }
- /* Only PAGE SIZE is supported yet */
- if (sectorsize != PAGE_SIZE) {
- btrfs_err(fs_info,
- "sectorsize %llu not supported yet, only support %lu",
- sectorsize, PAGE_SIZE);
- ret = -EINVAL;
- }
- if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
- nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
- btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
- ret = -EINVAL;
- }
- if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
- btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
- le32_to_cpu(sb->__unused_leafsize), nodesize);
- ret = -EINVAL;
- }
-
- /* Root alignment check */
- if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
- btrfs_warn(fs_info, "tree_root block unaligned: %llu",
- btrfs_super_root(sb));
- ret = -EINVAL;
- }
- if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
- btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
- btrfs_super_chunk_root(sb));
- ret = -EINVAL;
- }
- if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
- btrfs_warn(fs_info, "log_root block unaligned: %llu",
- btrfs_super_log_root(sb));
- ret = -EINVAL;
- }
-
- if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) != 0) {
- btrfs_err(fs_info,
- "dev_item UUID does not match fsid: %pU != %pU",
- fs_info->fsid, sb->dev_item.fsid);
- ret = -EINVAL;
- }
-
- /*
- * Hint to catch really bogus numbers, bitflips or so, more exact checks are
- * done later
- */
- if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
- btrfs_err(fs_info, "bytes_used is too small %llu",
- btrfs_super_bytes_used(sb));
- ret = -EINVAL;
- }
- if (!is_power_of_2(btrfs_super_stripesize(sb))) {
- btrfs_err(fs_info, "invalid stripesize %u",
- btrfs_super_stripesize(sb));
- ret = -EINVAL;
- }
- if (btrfs_super_num_devices(sb) > (1UL << 31))
- btrfs_warn(fs_info, "suspicious number of devices: %llu",
- btrfs_super_num_devices(sb));
- if (btrfs_super_num_devices(sb) == 0) {
- btrfs_err(fs_info, "number of devices is 0");
- ret = -EINVAL;
- }
-
- if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
- btrfs_err(fs_info, "super offset mismatch %llu != %u",
- btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
- ret = -EINVAL;
- }
-
- /*
- * Obvious sys_chunk_array corruptions, it must hold at least one key
- * and one chunk
- */
- if (btrfs_super_sys_array_size(sb) > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
- btrfs_err(fs_info, "system chunk array too big %u > %u",
- btrfs_super_sys_array_size(sb),
- BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
- ret = -EINVAL;
- }
- if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
- + sizeof(struct btrfs_chunk)) {
- btrfs_err(fs_info, "system chunk array too small %u < %zu",
- btrfs_super_sys_array_size(sb),
- sizeof(struct btrfs_disk_key)
- + sizeof(struct btrfs_chunk));
- ret = -EINVAL;
- }
-
- /*
- * The generation is a global counter, we'll trust it more than the others
- * but it's still possible that it's the one that's wrong.
- */
- if (btrfs_super_generation(sb) < btrfs_super_chunk_root_generation(sb))
- btrfs_warn(fs_info,
- "suspicious: generation < chunk_root_generation: %llu < %llu",
- btrfs_super_generation(sb),
- btrfs_super_chunk_root_generation(sb));
- if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
- && btrfs_super_cache_generation(sb) != (u64)-1)
- btrfs_warn(fs_info,
- "suspicious: generation < cache_generation: %llu < %llu",
- btrfs_super_generation(sb),
- btrfs_super_cache_generation(sb));
-
- return ret;
-}
-
static void btrfs_error_commit_super(struct btrfs_fs_info *fs_info)
{
mutex_lock(&fs_info->cleaner_mutex);
--
2.17.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 2/3] btrfs: Refactor btrfs_check_super_valid()
2018-05-11 5:35 [PATCH v3 0/3] btrfs: Add write time super block validation Qu Wenruo
2018-05-11 5:35 ` [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration Qu Wenruo
@ 2018-05-11 5:35 ` Qu Wenruo
2018-05-11 9:43 ` David Sterba
2018-05-11 5:35 ` [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk Qu Wenruo
2018-05-11 10:54 ` [PATCH v3 0/3] btrfs: Add write time super block validation David Sterba
3 siblings, 1 reply; 15+ messages in thread
From: Qu Wenruo @ 2018-05-11 5:35 UTC (permalink / raw)
To: linux-btrfs
Refactor btrfs_check_super_valid() by the ways:
1) Rename it to btrfs_validate_mount_super()
Now it's more obvious when the function should be called.
2) Extract core check routine into __validate_super()
So later write time check can reuse it, and if needed, we could also
use __validate_super() to check each super block.
3) Add more comment about btrfs_validate_mount_super()
Mostly of what it doesn't check and when it should be called.
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/disk-io.c | 29 +++++++++++++++++++++++++----
1 file changed, 25 insertions(+), 4 deletions(-)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index 13c5f90995aa..b981ecc4b6f9 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2440,9 +2440,19 @@ static int btrfs_read_roots(struct btrfs_fs_info *fs_info)
return ret;
}
-static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
+/*
+ * Real super block validation check
+ * NOTE: super csum type and incompat features will not be checked here.
+ *
+ * @sb: super block to check
+ * @mirror_num: the super block number to check its bytenr:
+ * 0 means the primary (1st) sb,
+ * 1 and 2 means 2nd and 3rd backup copy
+ * -1 means to skip bytenr check
+ */
+static int __validate_super(struct btrfs_fs_info *fs_info,
+ struct btrfs_super_block *sb, int mirror_num)
{
- struct btrfs_super_block *sb = fs_info->super_copy;
u64 nodesize = btrfs_super_nodesize(sb);
u64 sectorsize = btrfs_super_sectorsize(sb);
int ret = 0;
@@ -2545,7 +2555,8 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
ret = -EINVAL;
}
- if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
+ if (mirror_num >= 0 &&
+ btrfs_super_bytenr(sb) != btrfs_sb_offset(mirror_num)) {
btrfs_err(fs_info, "super offset mismatch %llu != %u",
btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
ret = -EINVAL;
@@ -2589,6 +2600,16 @@ static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
return ret;
}
+/*
+ * Check the validation of super block at mount time.
+ * Some checks already done by early mount, like csum type and incompat flags
+ * will be skipped.
+ */
+static int btrfs_validate_mount_super(struct btrfs_fs_info *fs_info)
+{
+ return __validate_super(fs_info, fs_info->super_copy, 0);
+}
+
int open_ctree(struct super_block *sb,
struct btrfs_fs_devices *fs_devices,
char *options)
@@ -2814,7 +2835,7 @@ int open_ctree(struct super_block *sb,
memcpy(fs_info->fsid, fs_info->super_copy->fsid, BTRFS_FSID_SIZE);
- ret = btrfs_check_super_valid(fs_info);
+ ret = btrfs_validate_mount_super(fs_info);
if (ret) {
btrfs_err(fs_info, "superblock contains fatal errors");
err = -EINVAL;
--
2.17.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk
2018-05-11 5:35 [PATCH v3 0/3] btrfs: Add write time super block validation Qu Wenruo
2018-05-11 5:35 ` [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration Qu Wenruo
2018-05-11 5:35 ` [PATCH v3 2/3] btrfs: Refactor btrfs_check_super_valid() Qu Wenruo
@ 2018-05-11 5:35 ` Qu Wenruo
2018-05-11 9:42 ` David Sterba
2018-05-11 10:56 ` David Sterba
2018-05-11 10:54 ` [PATCH v3 0/3] btrfs: Add write time super block validation David Sterba
3 siblings, 2 replies; 15+ messages in thread
From: Qu Wenruo @ 2018-05-11 5:35 UTC (permalink / raw)
To: linux-btrfs
There are already 2 reports about strangely corrupted super blocks,
where csum still matches but extra garbage gets slipped into super block.
The corruption would looks like:
------
superblock: bytenr=65536, device=/dev/sdc1
---------------------------------------------------------
csum_type 41700 (INVALID)
csum 0x3b252d3a [match]
bytenr 65536
flags 0x1
( WRITTEN )
magic _BHRfS_M [match]
...
incompat_flags 0x5b22400000000169
( MIXED_BACKREF |
COMPRESS_LZO |
BIG_METADATA |
EXTENDED_IREF |
SKINNY_METADATA |
unknown flag: 0x5b22400000000000 )
...
------
Or
------
superblock: bytenr=65536, device=/dev/mapper/x
---------------------------------------------------------
csum_type 35355 (INVALID)
csum_size 32
csum 0xf0dbeddd [match]
bytenr 65536
flags 0x1
( WRITTEN )
magic _BHRfS_M [match]
...
incompat_flags 0x176d200000000169
( MIXED_BACKREF |
COMPRESS_LZO |
BIG_METADATA |
EXTENDED_IREF |
SKINNY_METADATA |
unknown flag: 0x176d200000000000 )
------
Obviously, csum_type and incompat_flags get some garbage, but its csum
still matches, which means kernel calculates the csum based on corrupted
super block memory.
And after manually fixing these values, the filesystem is completely
healthy without any problem exposed by btrfs check.
Although the cause is still unknown, at least detect it and prevent further
corruption.
Reported-by: Ken Swenson <flat@imo.uto.moe>
Reported-by: Ben Parsons <9parsonsb@gmail.com>
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
fs/btrfs/disk-io.c | 39 +++++++++++++++++++++++++++++++++++++++
1 file changed, 39 insertions(+)
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index b981ecc4b6f9..985695074c51 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -2610,6 +2610,41 @@ static int btrfs_validate_mount_super(struct btrfs_fs_info *fs_info)
return __validate_super(fs_info, fs_info->super_copy, 0);
}
+/*
+ * Check the validation of super block at write time.
+ * Some checks like bytenr check will be skipped as their values will be
+ * overwritten soon.
+ * Extra checks like csum type and incompact flags will be executed here.
+ */
+static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
+ struct btrfs_super_block *sb)
+{
+ int ret;
+
+ ret = __validate_super(fs_info, sb, -1);
+ if (ret < 0)
+ goto out;
+ if (btrfs_super_csum_type(sb) != BTRFS_CSUM_TYPE_CRC32) {
+ ret = -EUCLEAN;
+ btrfs_err(fs_info, "invalid csum type, has %u want %u",
+ btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
+ goto out;
+ }
+ if (btrfs_super_incompat_flags(sb) & ~BTRFS_FEATURE_INCOMPAT_SUPP) {
+ ret = -EUCLEAN;
+ btrfs_err(fs_info,
+ "invalid incompact flags, has 0x%llu valid mask 0x%llu",
+ btrfs_super_incompat_flags(sb),
+ BTRFS_FEATURE_INCOMPAT_SUPP);
+ goto out;
+ }
+out:
+ if (ret < 0)
+ btrfs_err(fs_info,
+ "super block corruption detected before writing it to disk");
+ return ret;
+}
+
int open_ctree(struct super_block *sb,
struct btrfs_fs_devices *fs_devices,
char *options)
@@ -3730,6 +3765,10 @@ int write_all_supers(struct btrfs_fs_info *fs_info, int max_mirrors)
sb = fs_info->super_for_commit;
dev_item = &sb->dev_item;
+ ret = btrfs_validate_write_super(fs_info, sb);
+ if (ret < 0)
+ return ret;
+
mutex_lock(&fs_info->fs_devices->device_list_mutex);
head = &fs_info->fs_devices->devices;
max_errors = btrfs_super_num_devices(fs_info->super_copy) - 1;
--
2.17.0
^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-11 5:35 ` [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration Qu Wenruo
@ 2018-05-11 9:36 ` David Sterba
2018-05-11 10:37 ` David Sterba
2018-05-24 16:24 ` Anand Jain
1 sibling, 1 reply; 15+ messages in thread
From: David Sterba @ 2018-05-11 9:36 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Fri, May 11, 2018 at 01:35:25PM +0800, Qu Wenruo wrote:
> Just move btrfs_check_super_valid() before its single caller to avoid
> forward declaration.
Please don't move functions just to get rid of the forward declarations.
Moving functions to make them static or if they're in a wrong .c is OK,
but the extra forward declaration is not that bad and moving code
without any change just pollutest the git history. I'll drop the patch,
sorry.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk
2018-05-11 5:35 ` [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk Qu Wenruo
@ 2018-05-11 9:42 ` David Sterba
2018-05-11 10:56 ` David Sterba
1 sibling, 0 replies; 15+ messages in thread
From: David Sterba @ 2018-05-11 9:42 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Fri, May 11, 2018 at 01:35:27PM +0800, Qu Wenruo wrote:
> There are already 2 reports about strangely corrupted super blocks,
> where csum still matches but extra garbage gets slipped into super block.
>
> The corruption would looks like:
> ------
> superblock: bytenr=65536, device=/dev/sdc1
> ---------------------------------------------------------
> csum_type 41700 (INVALID)
> csum 0x3b252d3a [match]
> bytenr 65536
> flags 0x1
> ( WRITTEN )
> magic _BHRfS_M [match]
> ...
> incompat_flags 0x5b22400000000169
> ( MIXED_BACKREF |
> COMPRESS_LZO |
> BIG_METADATA |
> EXTENDED_IREF |
> SKINNY_METADATA |
> unknown flag: 0x5b22400000000000 )
> ...
> ------
> Or
> ------
> superblock: bytenr=65536, device=/dev/mapper/x
> ---------------------------------------------------------
> csum_type 35355 (INVALID)
> csum_size 32
> csum 0xf0dbeddd [match]
> bytenr 65536
> flags 0x1
> ( WRITTEN )
> magic _BHRfS_M [match]
> ...
> incompat_flags 0x176d200000000169
> ( MIXED_BACKREF |
> COMPRESS_LZO |
> BIG_METADATA |
> EXTENDED_IREF |
> SKINNY_METADATA |
> unknown flag: 0x176d200000000000 )
> ------
>
> Obviously, csum_type and incompat_flags get some garbage, but its csum
> still matches, which means kernel calculates the csum based on corrupted
> super block memory.
> And after manually fixing these values, the filesystem is completely
> healthy without any problem exposed by btrfs check.
>
> Although the cause is still unknown, at least detect it and prevent further
> corruption.
>
> Reported-by: Ken Swenson <flat@imo.uto.moe>
> Reported-by: Ben Parsons <9parsonsb@gmail.com>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/disk-io.c | 39 +++++++++++++++++++++++++++++++++++++++
> 1 file changed, 39 insertions(+)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index b981ecc4b6f9..985695074c51 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -2610,6 +2610,41 @@ static int btrfs_validate_mount_super(struct btrfs_fs_info *fs_info)
> return __validate_super(fs_info, fs_info->super_copy, 0);
> }
>
> +/*
> + * Check the validation of super block at write time.
> + * Some checks like bytenr check will be skipped as their values will be
> + * overwritten soon.
> + * Extra checks like csum type and incompact flags will be executed here.
> + */
> +static int btrfs_validate_write_super(struct btrfs_fs_info *fs_info,
> + struct btrfs_super_block *sb)
> +{
> + int ret;
> +
> + ret = __validate_super(fs_info, sb, -1);
> + if (ret < 0)
> + goto out;
> + if (btrfs_super_csum_type(sb) != BTRFS_CSUM_TYPE_CRC32) {
> + ret = -EUCLEAN;
> + btrfs_err(fs_info, "invalid csum type, has %u want %u",
> + btrfs_super_csum_type(sb), BTRFS_CSUM_TYPE_CRC32);
> + goto out;
> + }
> + if (btrfs_super_incompat_flags(sb) & ~BTRFS_FEATURE_INCOMPAT_SUPP) {
> + ret = -EUCLEAN;
> + btrfs_err(fs_info,
> + "invalid incompact flags, has 0x%llu valid mask 0x%llu",
> + btrfs_super_incompat_flags(sb),
> + BTRFS_FEATURE_INCOMPAT_SUPP);
I think you need (unsigned long long) here as
BTRFS_FEATURE_INCOMPAT_SUPP do not have a type. I'll fix that.
Reviewed-by: David Sterba <dsterba@suse.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 2/3] btrfs: Refactor btrfs_check_super_valid()
2018-05-11 5:35 ` [PATCH v3 2/3] btrfs: Refactor btrfs_check_super_valid() Qu Wenruo
@ 2018-05-11 9:43 ` David Sterba
0 siblings, 0 replies; 15+ messages in thread
From: David Sterba @ 2018-05-11 9:43 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Fri, May 11, 2018 at 01:35:26PM +0800, Qu Wenruo wrote:
> Refactor btrfs_check_super_valid() by the ways:
>
> 1) Rename it to btrfs_validate_mount_super()
> Now it's more obvious when the function should be called.
>
> 2) Extract core check routine into __validate_super()
> So later write time check can reuse it, and if needed, we could also
> use __validate_super() to check each super block.
As there's no validate_super without the underscores, I'd rather drop
them. Otherwise ok.
Reviewed-by: David Sterba <dsterba@suse.com>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-11 9:36 ` David Sterba
@ 2018-05-11 10:37 ` David Sterba
0 siblings, 0 replies; 15+ messages in thread
From: David Sterba @ 2018-05-11 10:37 UTC (permalink / raw)
To: David Sterba; +Cc: Qu Wenruo, linux-btrfs
On Fri, May 11, 2018 at 11:36:54AM +0200, David Sterba wrote:
> On Fri, May 11, 2018 at 01:35:25PM +0800, Qu Wenruo wrote:
> > Just move btrfs_check_super_valid() before its single caller to avoid
> > forward declaration.
>
> Please don't move functions just to get rid of the forward declarations.
>
> Moving functions to make them static or if they're in a wrong .c is OK,
> but the extra forward declaration is not that bad and moving code
> without any change just pollutest the git history. I'll drop the patch,
> sorry.
Hm, OK I now see why you did it. Fixing up the order of the related
static functions would need new forward declarations, so I'll apply the
patch after all.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 0/3] btrfs: Add write time super block validation
2018-05-11 5:35 [PATCH v3 0/3] btrfs: Add write time super block validation Qu Wenruo
` (2 preceding siblings ...)
2018-05-11 5:35 ` [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk Qu Wenruo
@ 2018-05-11 10:54 ` David Sterba
3 siblings, 0 replies; 15+ messages in thread
From: David Sterba @ 2018-05-11 10:54 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Fri, May 11, 2018 at 01:35:24PM +0800, Qu Wenruo wrote:
> This patchset can be fetched from github:
> https://github.com/adam900710/linux/tree/write_time_sb_check
>
> We have 2 reports about corrupted btrfs super block, which has some garbage
> in its super block, but otherwise it's completely fine and its csum even
> matches.
>
> This means we develop memory corruption during btrfs mount time.
> It's not clear whether it's caused by btrfs or some other kernel module,
> but at least let's do write time verification to catch such corruption
> early.
>
> Current design is to do 2 different checks at mount time and super write
> time.
> And for write time check, it only checks the template super block
> (fs_info->super_to_commit) other than each super blocks to be written to
> disk, mostly to avoid duplicated checks.
>
> Changelog:
> v2:
> Rename btrfs_check_super_valid() to btrfs_validate_super() suggested
> by Nikolay and David.
> v3:
> Add a new patch to move btrfs_check_super_valid() to avoid forward
> declaration.
> Refactor btrfs_check_super_valid() to provide better naming and
> function reusablity.
> Code style and comment update.
> Use 2 different functions, btrfs_validate_mount_super() and
> btrfs_validate_write_super(), for mount and write time super check.
Added as topic branch to next, I'm still targeting 4.18 with this
patchset so it'll end up in misc-next after some testing. Thanks.
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk
2018-05-11 5:35 ` [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk Qu Wenruo
2018-05-11 9:42 ` David Sterba
@ 2018-05-11 10:56 ` David Sterba
1 sibling, 0 replies; 15+ messages in thread
From: David Sterba @ 2018-05-11 10:56 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Fri, May 11, 2018 at 01:35:27PM +0800, Qu Wenruo wrote:
> +/*
> + * Check the validation of super block at write time.
> + * Some checks like bytenr check will be skipped as their values will be
> + * overwritten soon.
> + * Extra checks like csum type and incompact flags will be executed here.
^^^^^^^^^
I almost missed it, it's 'incompat', short from 'incompatibility'
> + if (btrfs_super_incompat_flags(sb) & ~BTRFS_FEATURE_INCOMPAT_SUPP) {
> + ret = -EUCLEAN;
> + btrfs_err(fs_info,
> + "invalid incompact flags, has 0x%llu valid mask 0x%llu",
^^^^^^^^^
Also fixed, as it's in a user visible string.
> + btrfs_super_incompat_flags(sb),
> + BTRFS_FEATURE_INCOMPAT_SUPP);
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-11 5:35 ` [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration Qu Wenruo
2018-05-11 9:36 ` David Sterba
@ 2018-05-24 16:24 ` Anand Jain
2018-05-24 17:42 ` David Sterba
2018-05-25 0:54 ` Qu Wenruo
1 sibling, 2 replies; 15+ messages in thread
From: Anand Jain @ 2018-05-24 16:24 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, David Sterba
On misc-next this patch is causing regression, the seed sprout
functionality test [1] (in the mailing list) fails.
[1]
[PATCH] fstests: btrfs: add seed sprout functionality test
more below..
On 05/11/2018 01:35 PM, Qu Wenruo wrote:
> Just move btrfs_check_super_valid() before its single caller to avoid
> forward declaration.
>
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
> fs/btrfs/disk-io.c | 299 ++++++++++++++++++++++-----------------------
> 1 file changed, 149 insertions(+), 150 deletions(-)
>
> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
> index 932ed61d9554..13c5f90995aa 100644
> --- a/fs/btrfs/disk-io.c
> +++ b/fs/btrfs/disk-io.c
> @@ -55,7 +55,6 @@
> static const struct extent_io_ops btree_extent_io_ops;
> static void end_workqueue_fn(struct btrfs_work *work);
> static void free_fs_root(struct btrfs_root *root);
> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info);
> static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
> static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
> struct btrfs_fs_info *fs_info);
> @@ -2441,6 +2440,155 @@ static int btrfs_read_roots(struct btrfs_fs_info *fs_info)
> return ret;
> }
>
> +static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
> +{
> + struct btrfs_super_block *sb = fs_info->super_copy;
> + u64 nodesize = btrfs_super_nodesize(sb);
> + u64 sectorsize = btrfs_super_sectorsize(sb);
> + int ret = 0;
> +
> + if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
> + btrfs_err(fs_info, "no valid FS found");
> + ret = -EINVAL;
> + }
> + if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
> + btrfs_err(fs_info, "unrecognized or unsupported super flag: %llu",
> + btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
> + ret = -EINVAL;
> + }
> + if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
> + btrfs_err(fs_info, "tree_root level too big: %d >= %d",
> + btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
> + ret = -EINVAL;
> + }
> + if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
> + btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
> + btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
> + ret = -EINVAL;
> + }
> + if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
> + btrfs_err(fs_info, "log_root level too big: %d >= %d",
> + btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
> + ret = -EINVAL;
> + }
> +
> + /*
> + * Check sectorsize and nodesize first, other check will need it.
> + * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
> + */
> + if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
> + sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
> + btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
> + ret = -EINVAL;
> + }
> + /* Only PAGE SIZE is supported yet */
> + if (sectorsize != PAGE_SIZE) {
> + btrfs_err(fs_info,
> + "sectorsize %llu not supported yet, only support %lu",
> + sectorsize, PAGE_SIZE);
> + ret = -EINVAL;
> + }
> + if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
> + nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
> + btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
> + ret = -EINVAL;
> + }
> + if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
> + btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
> + le32_to_cpu(sb->__unused_leafsize), nodesize);
> + ret = -EINVAL;
> + }
> +
> + /* Root alignment check */
> + if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
> + btrfs_warn(fs_info, "tree_root block unaligned: %llu",
> + btrfs_super_root(sb));
> + ret = -EINVAL;
> + }
> + if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
> + btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
> + btrfs_super_chunk_root(sb));
> + ret = -EINVAL;
> + }
> + if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
> + btrfs_warn(fs_info, "log_root block unaligned: %llu",
> + btrfs_super_log_root(sb));
> + ret = -EINVAL;
> + }
> +
> + if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) != 0) {
> + btrfs_err(fs_info,
> + "dev_item UUID does not match fsid: %pU != %pU",
> + fs_info->fsid, sb->dev_item.fsid);
> + ret = -EINVAL;
> + }
No. Not all devices will have the same fsid in case of seed-sprout
mounted devices.
[55090.664841] BTRFS error (device sdc): dev_item UUID does not match
fsid: 994fd365-9baf-4cbc-bcb2-ac65cba6a183 !=
2f459b44-def5-45d6-aa39-60d7d2350a6e
[55090.698052] BTRFS error (device sdc): super block corruption detected
before writing it to disk
[55090.700661] BTRFS warning (device sdc): Skipping commit of aborted
transaction.
[55090.701914] ------------[ cut here ]------------
[55090.702751] BTRFS: Transaction aborted (error -22)
[55090.703644] WARNING: CPU: 1 PID: 14960 at fs/btrfs/transaction.c:1836
cleanup_transaction+0x8a/0x270 [btrfs]
[55090.704597] Modules linked in: btrfs xor zstd_decompress
zstd_compress xxhash raid6_pq [last unloaded: xor]
[55090.704597] CPU: 1 PID: 14960 Comm: btrfs Tainted: G W
4.17.0-rc6+ #52
[55090.704597] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
VirtualBox 12/01/2006
[55090.704597] RIP: 0010:cleanup_transaction+0x8a/0x270 [btrfs]
[55090.704597] RSP: 0000:ffffa94a0155bb88 EFLAGS: 00010286
[55090.704597] RAX: 0000000000000000 RBX: ffff9ec4d7ad4000 RCX:
0000000000000000
[55090.704597] RDX: ffff9ec4dfd1d1f0 RSI: ffff9ec4dfd154b8 RDI:
ffff9ec4dfd154b8
[55090.704597] RBP: ffff9ec4da2dfe00 R08: 00000000000008af R09:
0000000000000000
[55090.704597] R10: ffffa94a0155bc10 R11: ffffffff92d977ad R12:
ffff9ec4d2c58410
[55090.704597] R13: 00000000ffffffea R14: 00000000ffffffea R15:
ffff9ec4d7ad4778
[55090.704597] FS: 00007f7ead00d8c0(0000) GS:ffff9ec4dfd00000(0000)
knlGS:0000000000000000
[55090.704597] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
[55090.704597] CR2: 00005565919f7ab0 CR3: 0000000117b12004 CR4:
00000000000606e0
[55090.704597] Call Trace:
[55090.704597] ? wait_woken+0x80/0x80
[55090.704597] btrfs_commit_transaction+0x71a/0x8b0 [btrfs]
[55090.704597] ? kobject_rename+0x112/0x150
[55090.704597] btrfs_init_new_device+0xa64/0xf60 [btrfs]
[55090.704597] ? unlazy_walk+0x32/0xa0
[55090.704597] ? btrfs_ioctl+0xde2/0x25d0 [btrfs]
[55090.704597] btrfs_ioctl+0xde2/0x25d0 [btrfs]
[55090.704597] ? selinux_inode_getattr+0x71/0x90
[55090.704597] ? _copy_to_user+0x22/0x30
[55090.704597] ? cp_new_stat+0x12c/0x160
[55090.704597] ? do_vfs_ioctl+0xa1/0x600
[55090.704597] ? proc_kprobes_optimization_handler+0x17a/0x1a0
[55090.704597] do_vfs_ioctl+0xa1/0x600
[55090.704597] ksys_ioctl+0x70/0x80
[55090.704597] __x64_sys_ioctl+0x16/0x20
[55090.704597] do_syscall_64+0x42/0xf0
[55090.704597] entry_SYSCALL_64_after_hwframe+0x44/0xa9
[55090.704597] RIP: 0033:0x7f7eac0ae277
[55090.704597] RSP: 002b:00007ffd85a76958 EFLAGS: 00000206 ORIG_RAX:
0000000000000010
[55090.704597] RAX: ffffffffffffffda RBX: 00007ffd85a77b38 RCX:
00007f7eac0ae277
[55090.704597] RDX: 00007ffd85a769a0 RSI: 000000005000940a RDI:
0000000000000003
[55090.704597] RBP: 0000000000000001 R08: 0000000000000000 R09:
0000000000001011
[55090.704597] R10: 000000000000018f R11: 0000000000000206 R12:
0000000000000000
[55090.704597] R13: 0000000000000002 R14: 000000000209e290 R15:
00007ffd85a77b40
[55090.704597] Code: 38 83 f8 01 0f 87 f5 01 00 00 f0 48 0f ba ab 88 0c
00 00 02 72 17 41 83 fd fb 74 11 44 89 ee 48 c7 c7 18 7b 1e c0 e8 e6 e6
10 d1 <0f> 0b 44 89 e9 4c 8d ab 70 07 00 00 ba 2c 07 00 00 48 c7 c6 f0
[55090.704597] ---[ end trace 09b3c3dd4f060772 ]---
[55090.753144] BTRFS: error (device sdc) in cleanup_transaction:1836:
errno=-22 unknown
[55090.761076] BTRFS info (device sdc): forced readonly
[55090.767639] BTRFS info (device sdc): delayed_refs has NO entry
[55090.854114] umount btrfs dev=sdb s_active=2 dir=runner
[55090.875108] umount btrfs dev=sdc s_active=2 dir=scratch
[55090.892160] BTRFS error (device sdc): cleaner transaction attach
returned -30
Thanks, Anand
> + /*
> + * Hint to catch really bogus numbers, bitflips or so, more exact checks are
> + * done later
> + */
> + if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
> + btrfs_err(fs_info, "bytes_used is too small %llu",
> + btrfs_super_bytes_used(sb));
> + ret = -EINVAL;
> + }
> + if (!is_power_of_2(btrfs_super_stripesize(sb))) {
> + btrfs_err(fs_info, "invalid stripesize %u",
> + btrfs_super_stripesize(sb));
> + ret = -EINVAL;
> + }
> + if (btrfs_super_num_devices(sb) > (1UL << 31))
> + btrfs_warn(fs_info, "suspicious number of devices: %llu",
> + btrfs_super_num_devices(sb));
> + if (btrfs_super_num_devices(sb) == 0) {
> + btrfs_err(fs_info, "number of devices is 0");
> + ret = -EINVAL;
> + }
> +
> + if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
> + btrfs_err(fs_info, "super offset mismatch %llu != %u",
> + btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
> + ret = -EINVAL;
> + }
> +
> + /*
> + * Obvious sys_chunk_array corruptions, it must hold at least one key
> + * and one chunk
> + */
> + if (btrfs_super_sys_array_size(sb) > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
> + btrfs_err(fs_info, "system chunk array too big %u > %u",
> + btrfs_super_sys_array_size(sb),
> + BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
> + ret = -EINVAL;
> + }
> + if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
> + + sizeof(struct btrfs_chunk)) {
> + btrfs_err(fs_info, "system chunk array too small %u < %zu",
> + btrfs_super_sys_array_size(sb),
> + sizeof(struct btrfs_disk_key)
> + + sizeof(struct btrfs_chunk));
> + ret = -EINVAL;
> + }
> +
> + /*
> + * The generation is a global counter, we'll trust it more than the others
> + * but it's still possible that it's the one that's wrong.
> + */
> + if (btrfs_super_generation(sb) < btrfs_super_chunk_root_generation(sb))
> + btrfs_warn(fs_info,
> + "suspicious: generation < chunk_root_generation: %llu < %llu",
> + btrfs_super_generation(sb),
> + btrfs_super_chunk_root_generation(sb));
> + if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
> + && btrfs_super_cache_generation(sb) != (u64)-1)
> + btrfs_warn(fs_info,
> + "suspicious: generation < cache_generation: %llu < %llu",
> + btrfs_super_generation(sb),
> + btrfs_super_cache_generation(sb));
> +
> + return ret;
> +}
> +
> int open_ctree(struct super_block *sb,
> struct btrfs_fs_devices *fs_devices,
> char *options)
> @@ -3972,155 +4120,6 @@ int btrfs_read_buffer(struct extent_buffer *buf, u64 parent_transid, int level,
> level, first_key);
> }
>
> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
> -{
> - struct btrfs_super_block *sb = fs_info->super_copy;
> - u64 nodesize = btrfs_super_nodesize(sb);
> - u64 sectorsize = btrfs_super_sectorsize(sb);
> - int ret = 0;
> -
> - if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
> - btrfs_err(fs_info, "no valid FS found");
> - ret = -EINVAL;
> - }
> - if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
> - btrfs_err(fs_info, "unrecognized or unsupported super flag: %llu",
> - btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
> - ret = -EINVAL;
> - }
> - if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
> - btrfs_err(fs_info, "tree_root level too big: %d >= %d",
> - btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
> - ret = -EINVAL;
> - }
> - if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
> - btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
> - btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
> - ret = -EINVAL;
> - }
> - if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
> - btrfs_err(fs_info, "log_root level too big: %d >= %d",
> - btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
> - ret = -EINVAL;
> - }
> -
> - /*
> - * Check sectorsize and nodesize first, other check will need it.
> - * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
> - */
> - if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
> - sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
> - btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
> - ret = -EINVAL;
> - }
> - /* Only PAGE SIZE is supported yet */
> - if (sectorsize != PAGE_SIZE) {
> - btrfs_err(fs_info,
> - "sectorsize %llu not supported yet, only support %lu",
> - sectorsize, PAGE_SIZE);
> - ret = -EINVAL;
> - }
> - if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
> - nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
> - btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
> - ret = -EINVAL;
> - }
> - if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
> - btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
> - le32_to_cpu(sb->__unused_leafsize), nodesize);
> - ret = -EINVAL;
> - }
> -
> - /* Root alignment check */
> - if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
> - btrfs_warn(fs_info, "tree_root block unaligned: %llu",
> - btrfs_super_root(sb));
> - ret = -EINVAL;
> - }
> - if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
> - btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
> - btrfs_super_chunk_root(sb));
> - ret = -EINVAL;
> - }
> - if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
> - btrfs_warn(fs_info, "log_root block unaligned: %llu",
> - btrfs_super_log_root(sb));
> - ret = -EINVAL;
> - }
> -
> - if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) != 0) {
> - btrfs_err(fs_info,
> - "dev_item UUID does not match fsid: %pU != %pU",
> - fs_info->fsid, sb->dev_item.fsid);
> - ret = -EINVAL;
> - }
> -
> - /*
> - * Hint to catch really bogus numbers, bitflips or so, more exact checks are
> - * done later
> - */
> - if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
> - btrfs_err(fs_info, "bytes_used is too small %llu",
> - btrfs_super_bytes_used(sb));
> - ret = -EINVAL;
> - }
> - if (!is_power_of_2(btrfs_super_stripesize(sb))) {
> - btrfs_err(fs_info, "invalid stripesize %u",
> - btrfs_super_stripesize(sb));
> - ret = -EINVAL;
> - }
> - if (btrfs_super_num_devices(sb) > (1UL << 31))
> - btrfs_warn(fs_info, "suspicious number of devices: %llu",
> - btrfs_super_num_devices(sb));
> - if (btrfs_super_num_devices(sb) == 0) {
> - btrfs_err(fs_info, "number of devices is 0");
> - ret = -EINVAL;
> - }
> -
> - if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
> - btrfs_err(fs_info, "super offset mismatch %llu != %u",
> - btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
> - ret = -EINVAL;
> - }
> -
> - /*
> - * Obvious sys_chunk_array corruptions, it must hold at least one key
> - * and one chunk
> - */
> - if (btrfs_super_sys_array_size(sb) > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
> - btrfs_err(fs_info, "system chunk array too big %u > %u",
> - btrfs_super_sys_array_size(sb),
> - BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
> - ret = -EINVAL;
> - }
> - if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
> - + sizeof(struct btrfs_chunk)) {
> - btrfs_err(fs_info, "system chunk array too small %u < %zu",
> - btrfs_super_sys_array_size(sb),
> - sizeof(struct btrfs_disk_key)
> - + sizeof(struct btrfs_chunk));
> - ret = -EINVAL;
> - }
> -
> - /*
> - * The generation is a global counter, we'll trust it more than the others
> - * but it's still possible that it's the one that's wrong.
> - */
> - if (btrfs_super_generation(sb) < btrfs_super_chunk_root_generation(sb))
> - btrfs_warn(fs_info,
> - "suspicious: generation < chunk_root_generation: %llu < %llu",
> - btrfs_super_generation(sb),
> - btrfs_super_chunk_root_generation(sb));
> - if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
> - && btrfs_super_cache_generation(sb) != (u64)-1)
> - btrfs_warn(fs_info,
> - "suspicious: generation < cache_generation: %llu < %llu",
> - btrfs_super_generation(sb),
> - btrfs_super_cache_generation(sb));
> -
> - return ret;
> -}
> -
> static void btrfs_error_commit_super(struct btrfs_fs_info *fs_info)
> {
> mutex_lock(&fs_info->cleaner_mutex);
>
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-24 16:24 ` Anand Jain
@ 2018-05-24 17:42 ` David Sterba
2018-05-25 0:54 ` Qu Wenruo
1 sibling, 0 replies; 15+ messages in thread
From: David Sterba @ 2018-05-24 17:42 UTC (permalink / raw)
To: Anand Jain; +Cc: Qu Wenruo, linux-btrfs, David Sterba
On Fri, May 25, 2018 at 12:24:10AM +0800, Anand Jain wrote:
> On misc-next this patch is causing regression, the seed sprout
> functionality test [1] (in the mailing list) fails.
This patch merly moves the function so it must have been broken before
that already.
> > + if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) != 0) {
> > + btrfs_err(fs_info,
> > + "dev_item UUID does not match fsid: %pU != %pU",
> > + fs_info->fsid, sb->dev_item.fsid);
> > + ret = -EINVAL;
> > + }
>
>
> No. Not all devices will have the same fsid in case of seed-sprout
> mounted devices.
So we need to check if the filesystem has seeding enabled and then
compare the correct uuids, right?
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-24 16:24 ` Anand Jain
2018-05-24 17:42 ` David Sterba
@ 2018-05-25 0:54 ` Qu Wenruo
2018-05-25 2:59 ` Qu Wenruo
1 sibling, 1 reply; 15+ messages in thread
From: Qu Wenruo @ 2018-05-25 0:54 UTC (permalink / raw)
To: Anand Jain, Qu Wenruo; +Cc: linux-btrfs, David Sterba
[-- Attachment #1.1: Type: text/plain, Size: 20890 bytes --]
On 2018年05月25日 00:24, Anand Jain wrote:
>
>
> On misc-next this patch is causing regression, the seed sprout
> functionality test [1] (in the mailing list) fails.
>
> [1]
> [PATCH] fstests: btrfs: add seed sprout functionality test
>
> more below..
>
> On 05/11/2018 01:35 PM, Qu Wenruo wrote:
>> Just move btrfs_check_super_valid() before its single caller to avoid
>> forward declaration.
>>
>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>> ---
>> fs/btrfs/disk-io.c | 299 ++++++++++++++++++++++-----------------------
>> 1 file changed, 149 insertions(+), 150 deletions(-)
>>
>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>> index 932ed61d9554..13c5f90995aa 100644
>> --- a/fs/btrfs/disk-io.c
>> +++ b/fs/btrfs/disk-io.c
>> @@ -55,7 +55,6 @@
>> static const struct extent_io_ops btree_extent_io_ops;
>> static void end_workqueue_fn(struct btrfs_work *work);
>> static void free_fs_root(struct btrfs_root *root);
>> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info);
>> static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
>> static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>> struct btrfs_fs_info *fs_info);
>> @@ -2441,6 +2440,155 @@ static int btrfs_read_roots(struct
>> btrfs_fs_info *fs_info)
>> return ret;
>> }
>> +static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
>> +{
>> + struct btrfs_super_block *sb = fs_info->super_copy;
>> + u64 nodesize = btrfs_super_nodesize(sb);
>> + u64 sectorsize = btrfs_super_sectorsize(sb);
>> + int ret = 0;
>> +
>> + if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
>> + btrfs_err(fs_info, "no valid FS found");
>> + ret = -EINVAL;
>> + }
>> + if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
>> + btrfs_err(fs_info, "unrecognized or unsupported super flag:
>> %llu",
>> + btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>> + ret = -EINVAL;
>> + }
>> + if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
>> + btrfs_err(fs_info, "tree_root level too big: %d >= %d",
>> + btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
>> + ret = -EINVAL;
>> + }
>> + if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
>> + btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
>> + btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
>> + ret = -EINVAL;
>> + }
>> + if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
>> + btrfs_err(fs_info, "log_root level too big: %d >= %d",
>> + btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
>> + ret = -EINVAL;
>> + }
>> +
>> + /*
>> + * Check sectorsize and nodesize first, other check will need it.
>> + * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
>> + */
>> + if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
>> + sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>> + btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
>> + ret = -EINVAL;
>> + }
>> + /* Only PAGE SIZE is supported yet */
>> + if (sectorsize != PAGE_SIZE) {
>> + btrfs_err(fs_info,
>> + "sectorsize %llu not supported yet, only support %lu",
>> + sectorsize, PAGE_SIZE);
>> + ret = -EINVAL;
>> + }
>> + if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
>> + nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>> + btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
>> + ret = -EINVAL;
>> + }
>> + if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
>> + btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
>> + le32_to_cpu(sb->__unused_leafsize), nodesize);
>> + ret = -EINVAL;
>> + }
>> +
>> + /* Root alignment check */
>> + if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
>> + btrfs_warn(fs_info, "tree_root block unaligned: %llu",
>> + btrfs_super_root(sb));
>> + ret = -EINVAL;
>> + }
>> + if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
>> + btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
>> + btrfs_super_chunk_root(sb));
>> + ret = -EINVAL;
>> + }
>> + if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
>> + btrfs_warn(fs_info, "log_root block unaligned: %llu",
>> + btrfs_super_log_root(sb));
>> + ret = -EINVAL;
>> + }
>> +
>> + if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) !=
>> 0) {
>> + btrfs_err(fs_info,
>> + "dev_item UUID does not match fsid: %pU != %pU",
>> + fs_info->fsid, sb->dev_item.fsid);
>> + ret = -EINVAL;
>> + }
>
>
> No. Not all devices will have the same fsid in case of seed-sprout
> mounted devices.
Yes, for seed device it's possible that seed device has different fsid.
But the problem here is, for seed device, it shouldn't get modified.
Thus its super block shouldn't get updated and written.
Or did I miss something?
Thanks,
Qu
>
> [55090.664841] BTRFS error (device sdc): dev_item UUID does not match
> fsid: 994fd365-9baf-4cbc-bcb2-ac65cba6a183 !=
> 2f459b44-def5-45d6-aa39-60d7d2350a6e
> [55090.698052] BTRFS error (device sdc): super block corruption detected
> before writing it to disk
> [55090.700661] BTRFS warning (device sdc): Skipping commit of aborted
> transaction.
> [55090.701914] ------------[ cut here ]------------
> [55090.702751] BTRFS: Transaction aborted (error -22)
> [55090.703644] WARNING: CPU: 1 PID: 14960 at fs/btrfs/transaction.c:1836
> cleanup_transaction+0x8a/0x270 [btrfs]
> [55090.704597] Modules linked in: btrfs xor zstd_decompress
> zstd_compress xxhash raid6_pq [last unloaded: xor]
> [55090.704597] CPU: 1 PID: 14960 Comm: btrfs Tainted: G W
> 4.17.0-rc6+ #52
> [55090.704597] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
> VirtualBox 12/01/2006
> [55090.704597] RIP: 0010:cleanup_transaction+0x8a/0x270 [btrfs]
> [55090.704597] RSP: 0000:ffffa94a0155bb88 EFLAGS: 00010286
> [55090.704597] RAX: 0000000000000000 RBX: ffff9ec4d7ad4000 RCX:
> 0000000000000000
> [55090.704597] RDX: ffff9ec4dfd1d1f0 RSI: ffff9ec4dfd154b8 RDI:
> ffff9ec4dfd154b8
> [55090.704597] RBP: ffff9ec4da2dfe00 R08: 00000000000008af R09:
> 0000000000000000
> [55090.704597] R10: ffffa94a0155bc10 R11: ffffffff92d977ad R12:
> ffff9ec4d2c58410
> [55090.704597] R13: 00000000ffffffea R14: 00000000ffffffea R15:
> ffff9ec4d7ad4778
> [55090.704597] FS: 00007f7ead00d8c0(0000) GS:ffff9ec4dfd00000(0000)
> knlGS:0000000000000000
> [55090.704597] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
> [55090.704597] CR2: 00005565919f7ab0 CR3: 0000000117b12004 CR4:
> 00000000000606e0
> [55090.704597] Call Trace:
> [55090.704597] ? wait_woken+0x80/0x80
> [55090.704597] btrfs_commit_transaction+0x71a/0x8b0 [btrfs]
> [55090.704597] ? kobject_rename+0x112/0x150
> [55090.704597] btrfs_init_new_device+0xa64/0xf60 [btrfs]
> [55090.704597] ? unlazy_walk+0x32/0xa0
> [55090.704597] ? btrfs_ioctl+0xde2/0x25d0 [btrfs]
> [55090.704597] btrfs_ioctl+0xde2/0x25d0 [btrfs]
> [55090.704597] ? selinux_inode_getattr+0x71/0x90
> [55090.704597] ? _copy_to_user+0x22/0x30
> [55090.704597] ? cp_new_stat+0x12c/0x160
> [55090.704597] ? do_vfs_ioctl+0xa1/0x600
> [55090.704597] ? proc_kprobes_optimization_handler+0x17a/0x1a0
> [55090.704597] do_vfs_ioctl+0xa1/0x600
> [55090.704597] ksys_ioctl+0x70/0x80
> [55090.704597] __x64_sys_ioctl+0x16/0x20
> [55090.704597] do_syscall_64+0x42/0xf0
> [55090.704597] entry_SYSCALL_64_after_hwframe+0x44/0xa9
> [55090.704597] RIP: 0033:0x7f7eac0ae277
> [55090.704597] RSP: 002b:00007ffd85a76958 EFLAGS: 00000206 ORIG_RAX:
> 0000000000000010
> [55090.704597] RAX: ffffffffffffffda RBX: 00007ffd85a77b38 RCX:
> 00007f7eac0ae277
> [55090.704597] RDX: 00007ffd85a769a0 RSI: 000000005000940a RDI:
> 0000000000000003
> [55090.704597] RBP: 0000000000000001 R08: 0000000000000000 R09:
> 0000000000001011
> [55090.704597] R10: 000000000000018f R11: 0000000000000206 R12:
> 0000000000000000
> [55090.704597] R13: 0000000000000002 R14: 000000000209e290 R15:
> 00007ffd85a77b40
> [55090.704597] Code: 38 83 f8 01 0f 87 f5 01 00 00 f0 48 0f ba ab 88 0c
> 00 00 02 72 17 41 83 fd fb 74 11 44 89 ee 48 c7 c7 18 7b 1e c0 e8 e6 e6
> 10 d1 <0f> 0b 44 89 e9 4c 8d ab 70 07 00 00 ba 2c 07 00 00 48 c7 c6 f0
> [55090.704597] ---[ end trace 09b3c3dd4f060772 ]---
> [55090.753144] BTRFS: error (device sdc) in cleanup_transaction:1836:
> errno=-22 unknown
> [55090.761076] BTRFS info (device sdc): forced readonly
> [55090.767639] BTRFS info (device sdc): delayed_refs has NO entry
> [55090.854114] umount btrfs dev=sdb s_active=2 dir=runner
> [55090.875108] umount btrfs dev=sdc s_active=2 dir=scratch
> [55090.892160] BTRFS error (device sdc): cleaner transaction attach
> returned -30
>
>
> Thanks, Anand
>
>
>> + /*
>> + * Hint to catch really bogus numbers, bitflips or so, more exact
>> checks are
>> + * done later
>> + */
>> + if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
>> + btrfs_err(fs_info, "bytes_used is too small %llu",
>> + btrfs_super_bytes_used(sb));
>> + ret = -EINVAL;
>> + }
>> + if (!is_power_of_2(btrfs_super_stripesize(sb))) {
>> + btrfs_err(fs_info, "invalid stripesize %u",
>> + btrfs_super_stripesize(sb));
>> + ret = -EINVAL;
>> + }
>> + if (btrfs_super_num_devices(sb) > (1UL << 31))
>> + btrfs_warn(fs_info, "suspicious number of devices: %llu",
>> + btrfs_super_num_devices(sb));
>> + if (btrfs_super_num_devices(sb) == 0) {
>> + btrfs_err(fs_info, "number of devices is 0");
>> + ret = -EINVAL;
>> + }
>> +
>> + if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
>> + btrfs_err(fs_info, "super offset mismatch %llu != %u",
>> + btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
>> + ret = -EINVAL;
>> + }
>> +
>> + /*
>> + * Obvious sys_chunk_array corruptions, it must hold at least one
>> key
>> + * and one chunk
>> + */
>> + if (btrfs_super_sys_array_size(sb) >
>> BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
>> + btrfs_err(fs_info, "system chunk array too big %u > %u",
>> + btrfs_super_sys_array_size(sb),
>> + BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
>> + ret = -EINVAL;
>> + }
>> + if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
>> + + sizeof(struct btrfs_chunk)) {
>> + btrfs_err(fs_info, "system chunk array too small %u < %zu",
>> + btrfs_super_sys_array_size(sb),
>> + sizeof(struct btrfs_disk_key)
>> + + sizeof(struct btrfs_chunk));
>> + ret = -EINVAL;
>> + }
>> +
>> + /*
>> + * The generation is a global counter, we'll trust it more than
>> the others
>> + * but it's still possible that it's the one that's wrong.
>> + */
>> + if (btrfs_super_generation(sb) <
>> btrfs_super_chunk_root_generation(sb))
>> + btrfs_warn(fs_info,
>> + "suspicious: generation < chunk_root_generation: %llu <
>> %llu",
>> + btrfs_super_generation(sb),
>> + btrfs_super_chunk_root_generation(sb));
>> + if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
>> + && btrfs_super_cache_generation(sb) != (u64)-1)
>> + btrfs_warn(fs_info,
>> + "suspicious: generation < cache_generation: %llu < %llu",
>> + btrfs_super_generation(sb),
>> + btrfs_super_cache_generation(sb));
>> +
>> + return ret;
>> +}
>> +
>> int open_ctree(struct super_block *sb,
>> struct btrfs_fs_devices *fs_devices,
>> char *options)
>> @@ -3972,155 +4120,6 @@ int btrfs_read_buffer(struct extent_buffer
>> *buf, u64 parent_transid, int level,
>> level, first_key);
>> }
>> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
>> -{
>> - struct btrfs_super_block *sb = fs_info->super_copy;
>> - u64 nodesize = btrfs_super_nodesize(sb);
>> - u64 sectorsize = btrfs_super_sectorsize(sb);
>> - int ret = 0;
>> -
>> - if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
>> - btrfs_err(fs_info, "no valid FS found");
>> - ret = -EINVAL;
>> - }
>> - if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
>> - btrfs_err(fs_info, "unrecognized or unsupported super flag:
>> %llu",
>> - btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>> - ret = -EINVAL;
>> - }
>> - if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
>> - btrfs_err(fs_info, "tree_root level too big: %d >= %d",
>> - btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
>> - ret = -EINVAL;
>> - }
>> - if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
>> - btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
>> - btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
>> - ret = -EINVAL;
>> - }
>> - if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
>> - btrfs_err(fs_info, "log_root level too big: %d >= %d",
>> - btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
>> - ret = -EINVAL;
>> - }
>> -
>> - /*
>> - * Check sectorsize and nodesize first, other check will need it.
>> - * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
>> - */
>> - if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
>> - sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>> - btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
>> - ret = -EINVAL;
>> - }
>> - /* Only PAGE SIZE is supported yet */
>> - if (sectorsize != PAGE_SIZE) {
>> - btrfs_err(fs_info,
>> - "sectorsize %llu not supported yet, only support %lu",
>> - sectorsize, PAGE_SIZE);
>> - ret = -EINVAL;
>> - }
>> - if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
>> - nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>> - btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
>> - ret = -EINVAL;
>> - }
>> - if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
>> - btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
>> - le32_to_cpu(sb->__unused_leafsize), nodesize);
>> - ret = -EINVAL;
>> - }
>> -
>> - /* Root alignment check */
>> - if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
>> - btrfs_warn(fs_info, "tree_root block unaligned: %llu",
>> - btrfs_super_root(sb));
>> - ret = -EINVAL;
>> - }
>> - if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
>> - btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
>> - btrfs_super_chunk_root(sb));
>> - ret = -EINVAL;
>> - }
>> - if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
>> - btrfs_warn(fs_info, "log_root block unaligned: %llu",
>> - btrfs_super_log_root(sb));
>> - ret = -EINVAL;
>> - }
>> -
>> - if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) !=
>> 0) {
>> - btrfs_err(fs_info,
>> - "dev_item UUID does not match fsid: %pU != %pU",
>> - fs_info->fsid, sb->dev_item.fsid);
>> - ret = -EINVAL;
>> - }
>> -
>> - /*
>> - * Hint to catch really bogus numbers, bitflips or so, more exact
>> checks are
>> - * done later
>> - */
>> - if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
>> - btrfs_err(fs_info, "bytes_used is too small %llu",
>> - btrfs_super_bytes_used(sb));
>> - ret = -EINVAL;
>> - }
>> - if (!is_power_of_2(btrfs_super_stripesize(sb))) {
>> - btrfs_err(fs_info, "invalid stripesize %u",
>> - btrfs_super_stripesize(sb));
>> - ret = -EINVAL;
>> - }
>> - if (btrfs_super_num_devices(sb) > (1UL << 31))
>> - btrfs_warn(fs_info, "suspicious number of devices: %llu",
>> - btrfs_super_num_devices(sb));
>> - if (btrfs_super_num_devices(sb) == 0) {
>> - btrfs_err(fs_info, "number of devices is 0");
>> - ret = -EINVAL;
>> - }
>> -
>> - if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
>> - btrfs_err(fs_info, "super offset mismatch %llu != %u",
>> - btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
>> - ret = -EINVAL;
>> - }
>> -
>> - /*
>> - * Obvious sys_chunk_array corruptions, it must hold at least one
>> key
>> - * and one chunk
>> - */
>> - if (btrfs_super_sys_array_size(sb) >
>> BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
>> - btrfs_err(fs_info, "system chunk array too big %u > %u",
>> - btrfs_super_sys_array_size(sb),
>> - BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
>> - ret = -EINVAL;
>> - }
>> - if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
>> - + sizeof(struct btrfs_chunk)) {
>> - btrfs_err(fs_info, "system chunk array too small %u < %zu",
>> - btrfs_super_sys_array_size(sb),
>> - sizeof(struct btrfs_disk_key)
>> - + sizeof(struct btrfs_chunk));
>> - ret = -EINVAL;
>> - }
>> -
>> - /*
>> - * The generation is a global counter, we'll trust it more than
>> the others
>> - * but it's still possible that it's the one that's wrong.
>> - */
>> - if (btrfs_super_generation(sb) <
>> btrfs_super_chunk_root_generation(sb))
>> - btrfs_warn(fs_info,
>> - "suspicious: generation < chunk_root_generation: %llu <
>> %llu",
>> - btrfs_super_generation(sb),
>> - btrfs_super_chunk_root_generation(sb));
>> - if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
>> - && btrfs_super_cache_generation(sb) != (u64)-1)
>> - btrfs_warn(fs_info,
>> - "suspicious: generation < cache_generation: %llu < %llu",
>> - btrfs_super_generation(sb),
>> - btrfs_super_cache_generation(sb));
>> -
>> - return ret;
>> -}
>> -
>> static void btrfs_error_commit_super(struct btrfs_fs_info *fs_info)
>> {
>> mutex_lock(&fs_info->cleaner_mutex);
>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-25 0:54 ` Qu Wenruo
@ 2018-05-25 2:59 ` Qu Wenruo
2018-05-25 3:13 ` Anand Jain
0 siblings, 1 reply; 15+ messages in thread
From: Qu Wenruo @ 2018-05-25 2:59 UTC (permalink / raw)
To: Anand Jain, Qu Wenruo; +Cc: linux-btrfs, David Sterba
[-- Attachment #1.1: Type: text/plain, Size: 21649 bytes --]
On 2018年05月25日 08:54, Qu Wenruo wrote:
>
>
> On 2018年05月25日 00:24, Anand Jain wrote:
>>
>>
>> On misc-next this patch is causing regression, the seed sprout
>> functionality test [1] (in the mailing list) fails.
>>
>> [1]
>> [PATCH] fstests: btrfs: add seed sprout functionality test
>>
>> more below..
>>
>> On 05/11/2018 01:35 PM, Qu Wenruo wrote:
>>> Just move btrfs_check_super_valid() before its single caller to avoid
>>> forward declaration.
>>>
>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>> ---
>>> fs/btrfs/disk-io.c | 299 ++++++++++++++++++++++-----------------------
>>> 1 file changed, 149 insertions(+), 150 deletions(-)
>>>
>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>> index 932ed61d9554..13c5f90995aa 100644
>>> --- a/fs/btrfs/disk-io.c
>>> +++ b/fs/btrfs/disk-io.c
>>> @@ -55,7 +55,6 @@
>>> static const struct extent_io_ops btree_extent_io_ops;
>>> static void end_workqueue_fn(struct btrfs_work *work);
>>> static void free_fs_root(struct btrfs_root *root);
>>> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info);
>>> static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
>>> static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>>> struct btrfs_fs_info *fs_info);
>>> @@ -2441,6 +2440,155 @@ static int btrfs_read_roots(struct
>>> btrfs_fs_info *fs_info)
>>> return ret;
>>> }
>>> +static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
>>> +{
>>> + struct btrfs_super_block *sb = fs_info->super_copy;
>>> + u64 nodesize = btrfs_super_nodesize(sb);
>>> + u64 sectorsize = btrfs_super_sectorsize(sb);
>>> + int ret = 0;
>>> +
>>> + if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
>>> + btrfs_err(fs_info, "no valid FS found");
>>> + ret = -EINVAL;
>>> + }
>>> + if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
>>> + btrfs_err(fs_info, "unrecognized or unsupported super flag:
>>> %llu",
>>> + btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>> + ret = -EINVAL;
>>> + }
>>> + if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>> + btrfs_err(fs_info, "tree_root level too big: %d >= %d",
>>> + btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
>>> + ret = -EINVAL;
>>> + }
>>> + if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>> + btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
>>> + btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
>>> + ret = -EINVAL;
>>> + }
>>> + if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>> + btrfs_err(fs_info, "log_root level too big: %d >= %d",
>>> + btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + /*
>>> + * Check sectorsize and nodesize first, other check will need it.
>>> + * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
>>> + */
>>> + if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
>>> + sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>> + btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
>>> + ret = -EINVAL;
>>> + }
>>> + /* Only PAGE SIZE is supported yet */
>>> + if (sectorsize != PAGE_SIZE) {
>>> + btrfs_err(fs_info,
>>> + "sectorsize %llu not supported yet, only support %lu",
>>> + sectorsize, PAGE_SIZE);
>>> + ret = -EINVAL;
>>> + }
>>> + if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
>>> + nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>> + btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
>>> + ret = -EINVAL;
>>> + }
>>> + if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
>>> + btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
>>> + le32_to_cpu(sb->__unused_leafsize), nodesize);
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + /* Root alignment check */
>>> + if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
>>> + btrfs_warn(fs_info, "tree_root block unaligned: %llu",
>>> + btrfs_super_root(sb));
>>> + ret = -EINVAL;
>>> + }
>>> + if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
>>> + btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
>>> + btrfs_super_chunk_root(sb));
>>> + ret = -EINVAL;
>>> + }
>>> + if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
>>> + btrfs_warn(fs_info, "log_root block unaligned: %llu",
>>> + btrfs_super_log_root(sb));
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) !=
>>> 0) {
>>> + btrfs_err(fs_info,
>>> + "dev_item UUID does not match fsid: %pU != %pU",
>>> + fs_info->fsid, sb->dev_item.fsid);
>>> + ret = -EINVAL;
>>> + }
>>
>>
>> No. Not all devices will have the same fsid in case of seed-sprout
>> mounted devices.
>
> Yes, for seed device it's possible that seed device has different fsid.
>
> But the problem here is, for seed device, it shouldn't get modified.
> Thus its super block shouldn't get updated and written.
>
> Or did I miss something?
OK, the problem is that we're using the old super block copy from seed
device.
The timming of checking superblock should be called after we have set
dev_item/fsid correct.
I'll update the patchset to handle it.
Thanks,
Qu
>
> Thanks,
> Qu
>
>>
>> [55090.664841] BTRFS error (device sdc): dev_item UUID does not match
>> fsid: 994fd365-9baf-4cbc-bcb2-ac65cba6a183 !=
>> 2f459b44-def5-45d6-aa39-60d7d2350a6e
>> [55090.698052] BTRFS error (device sdc): super block corruption detected
>> before writing it to disk
>> [55090.700661] BTRFS warning (device sdc): Skipping commit of aborted
>> transaction.
>> [55090.701914] ------------[ cut here ]------------
>> [55090.702751] BTRFS: Transaction aborted (error -22)
>> [55090.703644] WARNING: CPU: 1 PID: 14960 at fs/btrfs/transaction.c:1836
>> cleanup_transaction+0x8a/0x270 [btrfs]
>> [55090.704597] Modules linked in: btrfs xor zstd_decompress
>> zstd_compress xxhash raid6_pq [last unloaded: xor]
>> [55090.704597] CPU: 1 PID: 14960 Comm: btrfs Tainted: G W
>> 4.17.0-rc6+ #52
>> [55090.704597] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
>> VirtualBox 12/01/2006
>> [55090.704597] RIP: 0010:cleanup_transaction+0x8a/0x270 [btrfs]
>> [55090.704597] RSP: 0000:ffffa94a0155bb88 EFLAGS: 00010286
>> [55090.704597] RAX: 0000000000000000 RBX: ffff9ec4d7ad4000 RCX:
>> 0000000000000000
>> [55090.704597] RDX: ffff9ec4dfd1d1f0 RSI: ffff9ec4dfd154b8 RDI:
>> ffff9ec4dfd154b8
>> [55090.704597] RBP: ffff9ec4da2dfe00 R08: 00000000000008af R09:
>> 0000000000000000
>> [55090.704597] R10: ffffa94a0155bc10 R11: ffffffff92d977ad R12:
>> ffff9ec4d2c58410
>> [55090.704597] R13: 00000000ffffffea R14: 00000000ffffffea R15:
>> ffff9ec4d7ad4778
>> [55090.704597] FS: 00007f7ead00d8c0(0000) GS:ffff9ec4dfd00000(0000)
>> knlGS:0000000000000000
>> [55090.704597] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>> [55090.704597] CR2: 00005565919f7ab0 CR3: 0000000117b12004 CR4:
>> 00000000000606e0
>> [55090.704597] Call Trace:
>> [55090.704597] ? wait_woken+0x80/0x80
>> [55090.704597] btrfs_commit_transaction+0x71a/0x8b0 [btrfs]
>> [55090.704597] ? kobject_rename+0x112/0x150
>> [55090.704597] btrfs_init_new_device+0xa64/0xf60 [btrfs]
>> [55090.704597] ? unlazy_walk+0x32/0xa0
>> [55090.704597] ? btrfs_ioctl+0xde2/0x25d0 [btrfs]
>> [55090.704597] btrfs_ioctl+0xde2/0x25d0 [btrfs]
>> [55090.704597] ? selinux_inode_getattr+0x71/0x90
>> [55090.704597] ? _copy_to_user+0x22/0x30
>> [55090.704597] ? cp_new_stat+0x12c/0x160
>> [55090.704597] ? do_vfs_ioctl+0xa1/0x600
>> [55090.704597] ? proc_kprobes_optimization_handler+0x17a/0x1a0
>> [55090.704597] do_vfs_ioctl+0xa1/0x600
>> [55090.704597] ksys_ioctl+0x70/0x80
>> [55090.704597] __x64_sys_ioctl+0x16/0x20
>> [55090.704597] do_syscall_64+0x42/0xf0
>> [55090.704597] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>> [55090.704597] RIP: 0033:0x7f7eac0ae277
>> [55090.704597] RSP: 002b:00007ffd85a76958 EFLAGS: 00000206 ORIG_RAX:
>> 0000000000000010
>> [55090.704597] RAX: ffffffffffffffda RBX: 00007ffd85a77b38 RCX:
>> 00007f7eac0ae277
>> [55090.704597] RDX: 00007ffd85a769a0 RSI: 000000005000940a RDI:
>> 0000000000000003
>> [55090.704597] RBP: 0000000000000001 R08: 0000000000000000 R09:
>> 0000000000001011
>> [55090.704597] R10: 000000000000018f R11: 0000000000000206 R12:
>> 0000000000000000
>> [55090.704597] R13: 0000000000000002 R14: 000000000209e290 R15:
>> 00007ffd85a77b40
>> [55090.704597] Code: 38 83 f8 01 0f 87 f5 01 00 00 f0 48 0f ba ab 88 0c
>> 00 00 02 72 17 41 83 fd fb 74 11 44 89 ee 48 c7 c7 18 7b 1e c0 e8 e6 e6
>> 10 d1 <0f> 0b 44 89 e9 4c 8d ab 70 07 00 00 ba 2c 07 00 00 48 c7 c6 f0
>> [55090.704597] ---[ end trace 09b3c3dd4f060772 ]---
>> [55090.753144] BTRFS: error (device sdc) in cleanup_transaction:1836:
>> errno=-22 unknown
>> [55090.761076] BTRFS info (device sdc): forced readonly
>> [55090.767639] BTRFS info (device sdc): delayed_refs has NO entry
>> [55090.854114] umount btrfs dev=sdb s_active=2 dir=runner
>> [55090.875108] umount btrfs dev=sdc s_active=2 dir=scratch
>> [55090.892160] BTRFS error (device sdc): cleaner transaction attach
>> returned -30
>>
>>
>> Thanks, Anand
>>
>>
>>> + /*
>>> + * Hint to catch really bogus numbers, bitflips or so, more exact
>>> checks are
>>> + * done later
>>> + */
>>> + if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
>>> + btrfs_err(fs_info, "bytes_used is too small %llu",
>>> + btrfs_super_bytes_used(sb));
>>> + ret = -EINVAL;
>>> + }
>>> + if (!is_power_of_2(btrfs_super_stripesize(sb))) {
>>> + btrfs_err(fs_info, "invalid stripesize %u",
>>> + btrfs_super_stripesize(sb));
>>> + ret = -EINVAL;
>>> + }
>>> + if (btrfs_super_num_devices(sb) > (1UL << 31))
>>> + btrfs_warn(fs_info, "suspicious number of devices: %llu",
>>> + btrfs_super_num_devices(sb));
>>> + if (btrfs_super_num_devices(sb) == 0) {
>>> + btrfs_err(fs_info, "number of devices is 0");
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
>>> + btrfs_err(fs_info, "super offset mismatch %llu != %u",
>>> + btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + /*
>>> + * Obvious sys_chunk_array corruptions, it must hold at least one
>>> key
>>> + * and one chunk
>>> + */
>>> + if (btrfs_super_sys_array_size(sb) >
>>> BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
>>> + btrfs_err(fs_info, "system chunk array too big %u > %u",
>>> + btrfs_super_sys_array_size(sb),
>>> + BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
>>> + ret = -EINVAL;
>>> + }
>>> + if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
>>> + + sizeof(struct btrfs_chunk)) {
>>> + btrfs_err(fs_info, "system chunk array too small %u < %zu",
>>> + btrfs_super_sys_array_size(sb),
>>> + sizeof(struct btrfs_disk_key)
>>> + + sizeof(struct btrfs_chunk));
>>> + ret = -EINVAL;
>>> + }
>>> +
>>> + /*
>>> + * The generation is a global counter, we'll trust it more than
>>> the others
>>> + * but it's still possible that it's the one that's wrong.
>>> + */
>>> + if (btrfs_super_generation(sb) <
>>> btrfs_super_chunk_root_generation(sb))
>>> + btrfs_warn(fs_info,
>>> + "suspicious: generation < chunk_root_generation: %llu <
>>> %llu",
>>> + btrfs_super_generation(sb),
>>> + btrfs_super_chunk_root_generation(sb));
>>> + if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
>>> + && btrfs_super_cache_generation(sb) != (u64)-1)
>>> + btrfs_warn(fs_info,
>>> + "suspicious: generation < cache_generation: %llu < %llu",
>>> + btrfs_super_generation(sb),
>>> + btrfs_super_cache_generation(sb));
>>> +
>>> + return ret;
>>> +}
>>> +
>>> int open_ctree(struct super_block *sb,
>>> struct btrfs_fs_devices *fs_devices,
>>> char *options)
>>> @@ -3972,155 +4120,6 @@ int btrfs_read_buffer(struct extent_buffer
>>> *buf, u64 parent_transid, int level,
>>> level, first_key);
>>> }
>>> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
>>> -{
>>> - struct btrfs_super_block *sb = fs_info->super_copy;
>>> - u64 nodesize = btrfs_super_nodesize(sb);
>>> - u64 sectorsize = btrfs_super_sectorsize(sb);
>>> - int ret = 0;
>>> -
>>> - if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
>>> - btrfs_err(fs_info, "no valid FS found");
>>> - ret = -EINVAL;
>>> - }
>>> - if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
>>> - btrfs_err(fs_info, "unrecognized or unsupported super flag:
>>> %llu",
>>> - btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>> - ret = -EINVAL;
>>> - }
>>> - if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>> - btrfs_err(fs_info, "tree_root level too big: %d >= %d",
>>> - btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
>>> - ret = -EINVAL;
>>> - }
>>> - if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>> - btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
>>> - btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
>>> - ret = -EINVAL;
>>> - }
>>> - if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>> - btrfs_err(fs_info, "log_root level too big: %d >= %d",
>>> - btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
>>> - ret = -EINVAL;
>>> - }
>>> -
>>> - /*
>>> - * Check sectorsize and nodesize first, other check will need it.
>>> - * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
>>> - */
>>> - if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
>>> - sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>> - btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
>>> - ret = -EINVAL;
>>> - }
>>> - /* Only PAGE SIZE is supported yet */
>>> - if (sectorsize != PAGE_SIZE) {
>>> - btrfs_err(fs_info,
>>> - "sectorsize %llu not supported yet, only support %lu",
>>> - sectorsize, PAGE_SIZE);
>>> - ret = -EINVAL;
>>> - }
>>> - if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
>>> - nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>> - btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
>>> - ret = -EINVAL;
>>> - }
>>> - if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
>>> - btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
>>> - le32_to_cpu(sb->__unused_leafsize), nodesize);
>>> - ret = -EINVAL;
>>> - }
>>> -
>>> - /* Root alignment check */
>>> - if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
>>> - btrfs_warn(fs_info, "tree_root block unaligned: %llu",
>>> - btrfs_super_root(sb));
>>> - ret = -EINVAL;
>>> - }
>>> - if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
>>> - btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
>>> - btrfs_super_chunk_root(sb));
>>> - ret = -EINVAL;
>>> - }
>>> - if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
>>> - btrfs_warn(fs_info, "log_root block unaligned: %llu",
>>> - btrfs_super_log_root(sb));
>>> - ret = -EINVAL;
>>> - }
>>> -
>>> - if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) !=
>>> 0) {
>>> - btrfs_err(fs_info,
>>> - "dev_item UUID does not match fsid: %pU != %pU",
>>> - fs_info->fsid, sb->dev_item.fsid);
>>> - ret = -EINVAL;
>>> - }
>>> -
>>> - /*
>>> - * Hint to catch really bogus numbers, bitflips or so, more exact
>>> checks are
>>> - * done later
>>> - */
>>> - if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
>>> - btrfs_err(fs_info, "bytes_used is too small %llu",
>>> - btrfs_super_bytes_used(sb));
>>> - ret = -EINVAL;
>>> - }
>>> - if (!is_power_of_2(btrfs_super_stripesize(sb))) {
>>> - btrfs_err(fs_info, "invalid stripesize %u",
>>> - btrfs_super_stripesize(sb));
>>> - ret = -EINVAL;
>>> - }
>>> - if (btrfs_super_num_devices(sb) > (1UL << 31))
>>> - btrfs_warn(fs_info, "suspicious number of devices: %llu",
>>> - btrfs_super_num_devices(sb));
>>> - if (btrfs_super_num_devices(sb) == 0) {
>>> - btrfs_err(fs_info, "number of devices is 0");
>>> - ret = -EINVAL;
>>> - }
>>> -
>>> - if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
>>> - btrfs_err(fs_info, "super offset mismatch %llu != %u",
>>> - btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
>>> - ret = -EINVAL;
>>> - }
>>> -
>>> - /*
>>> - * Obvious sys_chunk_array corruptions, it must hold at least one
>>> key
>>> - * and one chunk
>>> - */
>>> - if (btrfs_super_sys_array_size(sb) >
>>> BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
>>> - btrfs_err(fs_info, "system chunk array too big %u > %u",
>>> - btrfs_super_sys_array_size(sb),
>>> - BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
>>> - ret = -EINVAL;
>>> - }
>>> - if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
>>> - + sizeof(struct btrfs_chunk)) {
>>> - btrfs_err(fs_info, "system chunk array too small %u < %zu",
>>> - btrfs_super_sys_array_size(sb),
>>> - sizeof(struct btrfs_disk_key)
>>> - + sizeof(struct btrfs_chunk));
>>> - ret = -EINVAL;
>>> - }
>>> -
>>> - /*
>>> - * The generation is a global counter, we'll trust it more than
>>> the others
>>> - * but it's still possible that it's the one that's wrong.
>>> - */
>>> - if (btrfs_super_generation(sb) <
>>> btrfs_super_chunk_root_generation(sb))
>>> - btrfs_warn(fs_info,
>>> - "suspicious: generation < chunk_root_generation: %llu <
>>> %llu",
>>> - btrfs_super_generation(sb),
>>> - btrfs_super_chunk_root_generation(sb));
>>> - if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
>>> - && btrfs_super_cache_generation(sb) != (u64)-1)
>>> - btrfs_warn(fs_info,
>>> - "suspicious: generation < cache_generation: %llu < %llu",
>>> - btrfs_super_generation(sb),
>>> - btrfs_super_cache_generation(sb));
>>> -
>>> - return ret;
>>> -}
>>> -
>>> static void btrfs_error_commit_super(struct btrfs_fs_info *fs_info)
>>> {
>>> mutex_lock(&fs_info->cleaner_mutex);
>>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]
^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration
2018-05-25 2:59 ` Qu Wenruo
@ 2018-05-25 3:13 ` Anand Jain
0 siblings, 0 replies; 15+ messages in thread
From: Anand Jain @ 2018-05-25 3:13 UTC (permalink / raw)
To: Qu Wenruo, Qu Wenruo; +Cc: linux-btrfs, David Sterba
On 05/25/2018 10:59 AM, Qu Wenruo wrote:
>
>
> On 2018年05月25日 08:54, Qu Wenruo wrote:
>>
>>
>> On 2018年05月25日 00:24, Anand Jain wrote:
>>>
>>>
>>> On misc-next this patch is causing regression, the seed sprout
>>> functionality test [1] (in the mailing list) fails.
>>>
>>> [1]
>>> [PATCH] fstests: btrfs: add seed sprout functionality test
>>>
>>> more below..
>>>
>>> On 05/11/2018 01:35 PM, Qu Wenruo wrote:
>>>> Just move btrfs_check_super_valid() before its single caller to avoid
>>>> forward declaration.
>>>>
>>>> Signed-off-by: Qu Wenruo <wqu@suse.com>
>>>> ---
>>>> fs/btrfs/disk-io.c | 299 ++++++++++++++++++++++-----------------------
>>>> 1 file changed, 149 insertions(+), 150 deletions(-)
>>>>
>>>> diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
>>>> index 932ed61d9554..13c5f90995aa 100644
>>>> --- a/fs/btrfs/disk-io.c
>>>> +++ b/fs/btrfs/disk-io.c
>>>> @@ -55,7 +55,6 @@
>>>> static const struct extent_io_ops btree_extent_io_ops;
>>>> static void end_workqueue_fn(struct btrfs_work *work);
>>>> static void free_fs_root(struct btrfs_root *root);
>>>> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info);
>>>> static void btrfs_destroy_ordered_extents(struct btrfs_root *root);
>>>> static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
>>>> struct btrfs_fs_info *fs_info);
>>>> @@ -2441,6 +2440,155 @@ static int btrfs_read_roots(struct
>>>> btrfs_fs_info *fs_info)
>>>> return ret;
>>>> }
>>>> +static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
>>>> +{
>>>> + struct btrfs_super_block *sb = fs_info->super_copy;
>>>> + u64 nodesize = btrfs_super_nodesize(sb);
>>>> + u64 sectorsize = btrfs_super_sectorsize(sb);
>>>> + int ret = 0;
>>>> +
>>>> + if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
>>>> + btrfs_err(fs_info, "no valid FS found");
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
>>>> + btrfs_err(fs_info, "unrecognized or unsupported super flag:
>>>> %llu",
>>>> + btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>>> + btrfs_err(fs_info, "tree_root level too big: %d >= %d",
>>>> + btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>>> + btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
>>>> + btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>>> + btrfs_err(fs_info, "log_root level too big: %d >= %d",
>>>> + btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
>>>> + ret = -EINVAL;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Check sectorsize and nodesize first, other check will need it.
>>>> + * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
>>>> + */
>>>> + if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
>>>> + sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>>> + btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
>>>> + ret = -EINVAL;
>>>> + }
>>>> + /* Only PAGE SIZE is supported yet */
>>>> + if (sectorsize != PAGE_SIZE) {
>>>> + btrfs_err(fs_info,
>>>> + "sectorsize %llu not supported yet, only support %lu",
>>>> + sectorsize, PAGE_SIZE);
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
>>>> + nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>>> + btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
>>>> + btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
>>>> + le32_to_cpu(sb->__unused_leafsize), nodesize);
>>>> + ret = -EINVAL;
>>>> + }
>>>> +
>>>> + /* Root alignment check */
>>>> + if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
>>>> + btrfs_warn(fs_info, "tree_root block unaligned: %llu",
>>>> + btrfs_super_root(sb));
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
>>>> + btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
>>>> + btrfs_super_chunk_root(sb));
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
>>>> + btrfs_warn(fs_info, "log_root block unaligned: %llu",
>>>> + btrfs_super_log_root(sb));
>>>> + ret = -EINVAL;
>>>> + }
>>>> +
>>>> + if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) !=
>>>> 0) {
>>>> + btrfs_err(fs_info,
>>>> + "dev_item UUID does not match fsid: %pU != %pU",
>>>> + fs_info->fsid, sb->dev_item.fsid);
>>>> + ret = -EINVAL;
>>>> + }
>>>
>>>
>>> No. Not all devices will have the same fsid in case of seed-sprout
>>> mounted devices.
>>
>> Yes, for seed device it's possible that seed device has different fsid.
>>
>> But the problem here is, for seed device, it shouldn't get modified.
>> Thus its super block shouldn't get updated and written.
>>
>> Or did I miss something?
>
> OK, the problem is that we're using the old super block copy from seed
> device.
>
> The timming of checking superblock should be called after we have set
> dev_item/fsid correct.
Yep further below. We don't write superblock at all to the seed device
as its readonly device.
----------------
write_all_supers()
::
list_for_each_entry(dev, head, dev_list) {
if (!dev->bdev) {
total_errors++;
continue;
}
if (!test_bit(BTRFS_DEV_STATE_IN_FS_METADATA,
&dev->dev_state) ||
!test_bit(BTRFS_DEV_STATE_WRITEABLE, &dev->dev_state))
continue;
::
ret = write_dev_supers(dev, sb, max_mirrors);
if (ret)
total_errors++;
----------------
Thanks, Anand
> I'll update the patchset to handle it.
>
> Thanks,
> Qu
>
>>
>> Thanks,
>> Qu
>>
>>>
>>> [55090.664841] BTRFS error (device sdc): dev_item UUID does not match
>>> fsid: 994fd365-9baf-4cbc-bcb2-ac65cba6a183 !=
>>> 2f459b44-def5-45d6-aa39-60d7d2350a6e
>>> [55090.698052] BTRFS error (device sdc): super block corruption detected
>>> before writing it to disk
>>> [55090.700661] BTRFS warning (device sdc): Skipping commit of aborted
>>> transaction.
>>> [55090.701914] ------------[ cut here ]------------
>>> [55090.702751] BTRFS: Transaction aborted (error -22)
>>> [55090.703644] WARNING: CPU: 1 PID: 14960 at fs/btrfs/transaction.c:1836
>>> cleanup_transaction+0x8a/0x270 [btrfs]
>>> [55090.704597] Modules linked in: btrfs xor zstd_decompress
>>> zstd_compress xxhash raid6_pq [last unloaded: xor]
>>> [55090.704597] CPU: 1 PID: 14960 Comm: btrfs Tainted: G W
>>> 4.17.0-rc6+ #52
>>> [55090.704597] Hardware name: innotek GmbH VirtualBox/VirtualBox, BIOS
>>> VirtualBox 12/01/2006
>>> [55090.704597] RIP: 0010:cleanup_transaction+0x8a/0x270 [btrfs]
>>> [55090.704597] RSP: 0000:ffffa94a0155bb88 EFLAGS: 00010286
>>> [55090.704597] RAX: 0000000000000000 RBX: ffff9ec4d7ad4000 RCX:
>>> 0000000000000000
>>> [55090.704597] RDX: ffff9ec4dfd1d1f0 RSI: ffff9ec4dfd154b8 RDI:
>>> ffff9ec4dfd154b8
>>> [55090.704597] RBP: ffff9ec4da2dfe00 R08: 00000000000008af R09:
>>> 0000000000000000
>>> [55090.704597] R10: ffffa94a0155bc10 R11: ffffffff92d977ad R12:
>>> ffff9ec4d2c58410
>>> [55090.704597] R13: 00000000ffffffea R14: 00000000ffffffea R15:
>>> ffff9ec4d7ad4778
>>> [55090.704597] FS: 00007f7ead00d8c0(0000) GS:ffff9ec4dfd00000(0000)
>>> knlGS:0000000000000000
>>> [55090.704597] CS: 0010 DS: 0000 ES: 0000 CR0: 0000000080050033
>>> [55090.704597] CR2: 00005565919f7ab0 CR3: 0000000117b12004 CR4:
>>> 00000000000606e0
>>> [55090.704597] Call Trace:
>>> [55090.704597] ? wait_woken+0x80/0x80
>>> [55090.704597] btrfs_commit_transaction+0x71a/0x8b0 [btrfs]
>>> [55090.704597] ? kobject_rename+0x112/0x150
>>> [55090.704597] btrfs_init_new_device+0xa64/0xf60 [btrfs]
>>> [55090.704597] ? unlazy_walk+0x32/0xa0
>>> [55090.704597] ? btrfs_ioctl+0xde2/0x25d0 [btrfs]
>>> [55090.704597] btrfs_ioctl+0xde2/0x25d0 [btrfs]
>>> [55090.704597] ? selinux_inode_getattr+0x71/0x90
>>> [55090.704597] ? _copy_to_user+0x22/0x30
>>> [55090.704597] ? cp_new_stat+0x12c/0x160
>>> [55090.704597] ? do_vfs_ioctl+0xa1/0x600
>>> [55090.704597] ? proc_kprobes_optimization_handler+0x17a/0x1a0
>>> [55090.704597] do_vfs_ioctl+0xa1/0x600
>>> [55090.704597] ksys_ioctl+0x70/0x80
>>> [55090.704597] __x64_sys_ioctl+0x16/0x20
>>> [55090.704597] do_syscall_64+0x42/0xf0
>>> [55090.704597] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>>> [55090.704597] RIP: 0033:0x7f7eac0ae277
>>> [55090.704597] RSP: 002b:00007ffd85a76958 EFLAGS: 00000206 ORIG_RAX:
>>> 0000000000000010
>>> [55090.704597] RAX: ffffffffffffffda RBX: 00007ffd85a77b38 RCX:
>>> 00007f7eac0ae277
>>> [55090.704597] RDX: 00007ffd85a769a0 RSI: 000000005000940a RDI:
>>> 0000000000000003
>>> [55090.704597] RBP: 0000000000000001 R08: 0000000000000000 R09:
>>> 0000000000001011
>>> [55090.704597] R10: 000000000000018f R11: 0000000000000206 R12:
>>> 0000000000000000
>>> [55090.704597] R13: 0000000000000002 R14: 000000000209e290 R15:
>>> 00007ffd85a77b40
>>> [55090.704597] Code: 38 83 f8 01 0f 87 f5 01 00 00 f0 48 0f ba ab 88 0c
>>> 00 00 02 72 17 41 83 fd fb 74 11 44 89 ee 48 c7 c7 18 7b 1e c0 e8 e6 e6
>>> 10 d1 <0f> 0b 44 89 e9 4c 8d ab 70 07 00 00 ba 2c 07 00 00 48 c7 c6 f0
>>> [55090.704597] ---[ end trace 09b3c3dd4f060772 ]---
>>> [55090.753144] BTRFS: error (device sdc) in cleanup_transaction:1836:
>>> errno=-22 unknown
>>> [55090.761076] BTRFS info (device sdc): forced readonly
>>> [55090.767639] BTRFS info (device sdc): delayed_refs has NO entry
>>> [55090.854114] umount btrfs dev=sdb s_active=2 dir=runner
>>> [55090.875108] umount btrfs dev=sdc s_active=2 dir=scratch
>>> [55090.892160] BTRFS error (device sdc): cleaner transaction attach
>>> returned -30
>>>
>>>
>>> Thanks, Anand
>>>
>>>
>>>> + /*
>>>> + * Hint to catch really bogus numbers, bitflips or so, more exact
>>>> checks are
>>>> + * done later
>>>> + */
>>>> + if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
>>>> + btrfs_err(fs_info, "bytes_used is too small %llu",
>>>> + btrfs_super_bytes_used(sb));
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (!is_power_of_2(btrfs_super_stripesize(sb))) {
>>>> + btrfs_err(fs_info, "invalid stripesize %u",
>>>> + btrfs_super_stripesize(sb));
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (btrfs_super_num_devices(sb) > (1UL << 31))
>>>> + btrfs_warn(fs_info, "suspicious number of devices: %llu",
>>>> + btrfs_super_num_devices(sb));
>>>> + if (btrfs_super_num_devices(sb) == 0) {
>>>> + btrfs_err(fs_info, "number of devices is 0");
>>>> + ret = -EINVAL;
>>>> + }
>>>> +
>>>> + if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
>>>> + btrfs_err(fs_info, "super offset mismatch %llu != %u",
>>>> + btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
>>>> + ret = -EINVAL;
>>>> + }
>>>> +
>>>> + /*
>>>> + * Obvious sys_chunk_array corruptions, it must hold at least one
>>>> key
>>>> + * and one chunk
>>>> + */
>>>> + if (btrfs_super_sys_array_size(sb) >
>>>> BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
>>>> + btrfs_err(fs_info, "system chunk array too big %u > %u",
>>>> + btrfs_super_sys_array_size(sb),
>>>> + BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
>>>> + ret = -EINVAL;
>>>> + }
>>>> + if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
>>>> + + sizeof(struct btrfs_chunk)) {
>>>> + btrfs_err(fs_info, "system chunk array too small %u < %zu",
>>>> + btrfs_super_sys_array_size(sb),
>>>> + sizeof(struct btrfs_disk_key)
>>>> + + sizeof(struct btrfs_chunk));
>>>> + ret = -EINVAL;
>>>> + }
>>>> +
>>>> + /*
>>>> + * The generation is a global counter, we'll trust it more than
>>>> the others
>>>> + * but it's still possible that it's the one that's wrong.
>>>> + */
>>>> + if (btrfs_super_generation(sb) <
>>>> btrfs_super_chunk_root_generation(sb))
>>>> + btrfs_warn(fs_info,
>>>> + "suspicious: generation < chunk_root_generation: %llu <
>>>> %llu",
>>>> + btrfs_super_generation(sb),
>>>> + btrfs_super_chunk_root_generation(sb));
>>>> + if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
>>>> + && btrfs_super_cache_generation(sb) != (u64)-1)
>>>> + btrfs_warn(fs_info,
>>>> + "suspicious: generation < cache_generation: %llu < %llu",
>>>> + btrfs_super_generation(sb),
>>>> + btrfs_super_cache_generation(sb));
>>>> +
>>>> + return ret;
>>>> +}
>>>> +
>>>> int open_ctree(struct super_block *sb,
>>>> struct btrfs_fs_devices *fs_devices,
>>>> char *options)
>>>> @@ -3972,155 +4120,6 @@ int btrfs_read_buffer(struct extent_buffer
>>>> *buf, u64 parent_transid, int level,
>>>> level, first_key);
>>>> }
>>>> -static int btrfs_check_super_valid(struct btrfs_fs_info *fs_info)
>>>> -{
>>>> - struct btrfs_super_block *sb = fs_info->super_copy;
>>>> - u64 nodesize = btrfs_super_nodesize(sb);
>>>> - u64 sectorsize = btrfs_super_sectorsize(sb);
>>>> - int ret = 0;
>>>> -
>>>> - if (btrfs_super_magic(sb) != BTRFS_MAGIC) {
>>>> - btrfs_err(fs_info, "no valid FS found");
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP) {
>>>> - btrfs_err(fs_info, "unrecognized or unsupported super flag:
>>>> %llu",
>>>> - btrfs_super_flags(sb) & ~BTRFS_SUPER_FLAG_SUPP);
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (btrfs_super_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>>> - btrfs_err(fs_info, "tree_root level too big: %d >= %d",
>>>> - btrfs_super_root_level(sb), BTRFS_MAX_LEVEL);
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (btrfs_super_chunk_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>>> - btrfs_err(fs_info, "chunk_root level too big: %d >= %d",
>>>> - btrfs_super_chunk_root_level(sb), BTRFS_MAX_LEVEL);
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (btrfs_super_log_root_level(sb) >= BTRFS_MAX_LEVEL) {
>>>> - btrfs_err(fs_info, "log_root level too big: %d >= %d",
>>>> - btrfs_super_log_root_level(sb), BTRFS_MAX_LEVEL);
>>>> - ret = -EINVAL;
>>>> - }
>>>> -
>>>> - /*
>>>> - * Check sectorsize and nodesize first, other check will need it.
>>>> - * Check all possible sectorsize(4K, 8K, 16K, 32K, 64K) here.
>>>> - */
>>>> - if (!is_power_of_2(sectorsize) || sectorsize < 4096 ||
>>>> - sectorsize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>>> - btrfs_err(fs_info, "invalid sectorsize %llu", sectorsize);
>>>> - ret = -EINVAL;
>>>> - }
>>>> - /* Only PAGE SIZE is supported yet */
>>>> - if (sectorsize != PAGE_SIZE) {
>>>> - btrfs_err(fs_info,
>>>> - "sectorsize %llu not supported yet, only support %lu",
>>>> - sectorsize, PAGE_SIZE);
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (!is_power_of_2(nodesize) || nodesize < sectorsize ||
>>>> - nodesize > BTRFS_MAX_METADATA_BLOCKSIZE) {
>>>> - btrfs_err(fs_info, "invalid nodesize %llu", nodesize);
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (nodesize != le32_to_cpu(sb->__unused_leafsize)) {
>>>> - btrfs_err(fs_info, "invalid leafsize %u, should be %llu",
>>>> - le32_to_cpu(sb->__unused_leafsize), nodesize);
>>>> - ret = -EINVAL;
>>>> - }
>>>> -
>>>> - /* Root alignment check */
>>>> - if (!IS_ALIGNED(btrfs_super_root(sb), sectorsize)) {
>>>> - btrfs_warn(fs_info, "tree_root block unaligned: %llu",
>>>> - btrfs_super_root(sb));
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (!IS_ALIGNED(btrfs_super_chunk_root(sb), sectorsize)) {
>>>> - btrfs_warn(fs_info, "chunk_root block unaligned: %llu",
>>>> - btrfs_super_chunk_root(sb));
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (!IS_ALIGNED(btrfs_super_log_root(sb), sectorsize)) {
>>>> - btrfs_warn(fs_info, "log_root block unaligned: %llu",
>>>> - btrfs_super_log_root(sb));
>>>> - ret = -EINVAL;
>>>> - }
>>>> -
>>>> - if (memcmp(fs_info->fsid, sb->dev_item.fsid, BTRFS_FSID_SIZE) !=
>>>> 0) {
>>>> - btrfs_err(fs_info,
>>>> - "dev_item UUID does not match fsid: %pU != %pU",
>>>> - fs_info->fsid, sb->dev_item.fsid);
>>>> - ret = -EINVAL;
>>>> - }
>>>> -
>>>> - /*
>>>> - * Hint to catch really bogus numbers, bitflips or so, more exact
>>>> checks are
>>>> - * done later
>>>> - */
>>>> - if (btrfs_super_bytes_used(sb) < 6 * btrfs_super_nodesize(sb)) {
>>>> - btrfs_err(fs_info, "bytes_used is too small %llu",
>>>> - btrfs_super_bytes_used(sb));
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (!is_power_of_2(btrfs_super_stripesize(sb))) {
>>>> - btrfs_err(fs_info, "invalid stripesize %u",
>>>> - btrfs_super_stripesize(sb));
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (btrfs_super_num_devices(sb) > (1UL << 31))
>>>> - btrfs_warn(fs_info, "suspicious number of devices: %llu",
>>>> - btrfs_super_num_devices(sb));
>>>> - if (btrfs_super_num_devices(sb) == 0) {
>>>> - btrfs_err(fs_info, "number of devices is 0");
>>>> - ret = -EINVAL;
>>>> - }
>>>> -
>>>> - if (btrfs_super_bytenr(sb) != BTRFS_SUPER_INFO_OFFSET) {
>>>> - btrfs_err(fs_info, "super offset mismatch %llu != %u",
>>>> - btrfs_super_bytenr(sb), BTRFS_SUPER_INFO_OFFSET);
>>>> - ret = -EINVAL;
>>>> - }
>>>> -
>>>> - /*
>>>> - * Obvious sys_chunk_array corruptions, it must hold at least one
>>>> key
>>>> - * and one chunk
>>>> - */
>>>> - if (btrfs_super_sys_array_size(sb) >
>>>> BTRFS_SYSTEM_CHUNK_ARRAY_SIZE) {
>>>> - btrfs_err(fs_info, "system chunk array too big %u > %u",
>>>> - btrfs_super_sys_array_size(sb),
>>>> - BTRFS_SYSTEM_CHUNK_ARRAY_SIZE);
>>>> - ret = -EINVAL;
>>>> - }
>>>> - if (btrfs_super_sys_array_size(sb) < sizeof(struct btrfs_disk_key)
>>>> - + sizeof(struct btrfs_chunk)) {
>>>> - btrfs_err(fs_info, "system chunk array too small %u < %zu",
>>>> - btrfs_super_sys_array_size(sb),
>>>> - sizeof(struct btrfs_disk_key)
>>>> - + sizeof(struct btrfs_chunk));
>>>> - ret = -EINVAL;
>>>> - }
>>>> -
>>>> - /*
>>>> - * The generation is a global counter, we'll trust it more than
>>>> the others
>>>> - * but it's still possible that it's the one that's wrong.
>>>> - */
>>>> - if (btrfs_super_generation(sb) <
>>>> btrfs_super_chunk_root_generation(sb))
>>>> - btrfs_warn(fs_info,
>>>> - "suspicious: generation < chunk_root_generation: %llu <
>>>> %llu",
>>>> - btrfs_super_generation(sb),
>>>> - btrfs_super_chunk_root_generation(sb));
>>>> - if (btrfs_super_generation(sb) < btrfs_super_cache_generation(sb)
>>>> - && btrfs_super_cache_generation(sb) != (u64)-1)
>>>> - btrfs_warn(fs_info,
>>>> - "suspicious: generation < cache_generation: %llu < %llu",
>>>> - btrfs_super_generation(sb),
>>>> - btrfs_super_cache_generation(sb));
>>>> -
>>>> - return ret;
>>>> -}
>>>> -
>>>> static void btrfs_error_commit_super(struct btrfs_fs_info *fs_info)
>>>> {
>>>> mutex_lock(&fs_info->cleaner_mutex);
>>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
>>> the body of a message to majordomo@vger.kernel.org
>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
>>
>
^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2018-05-25 3:10 UTC | newest]
Thread overview: 15+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-05-11 5:35 [PATCH v3 0/3] btrfs: Add write time super block validation Qu Wenruo
2018-05-11 5:35 ` [PATCH v3 1/3] btrfs: Move btrfs_check_super_valid() to avoid forward declaration Qu Wenruo
2018-05-11 9:36 ` David Sterba
2018-05-11 10:37 ` David Sterba
2018-05-24 16:24 ` Anand Jain
2018-05-24 17:42 ` David Sterba
2018-05-25 0:54 ` Qu Wenruo
2018-05-25 2:59 ` Qu Wenruo
2018-05-25 3:13 ` Anand Jain
2018-05-11 5:35 ` [PATCH v3 2/3] btrfs: Refactor btrfs_check_super_valid() Qu Wenruo
2018-05-11 9:43 ` David Sterba
2018-05-11 5:35 ` [PATCH v3 3/3] btrfs: Do super block verification before writing it to disk Qu Wenruo
2018-05-11 9:42 ` David Sterba
2018-05-11 10:56 ` David Sterba
2018-05-11 10:54 ` [PATCH v3 0/3] btrfs: Add write time super block validation David Sterba
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.