diff mbox series

[2/3] btrfs-progs: add extra chunk alignment checks

Message ID cd582deb540064fbf01b10f0651f00809b643e2e.1705375819.git.wqu@suse.com (mailing list archive)
State New, archived
Headers show
Series btrfs-progs: make convert to generate chunks aligned to stripe boundary | expand

Commit Message

Qu Wenruo Jan. 16, 2024, 3:31 a.m. UTC
Recently we have a scrub use-after-free caused by unaligned chunk
length, although the fix is submitted, we may want to do extra checks
for a chunk's alignment.

This patch would add such check for the starting bytenr and length of a
chunk, to make sure they are properly aligned to 64K stripe boundary.

By default, the check would only lead to a warning but not treated as an
error, as we expect kernel to handle such unalignment without any
problem.

But if the new debug environmental variable,
BTRFS_PROGS_DEBUG_STRICT_CHUNK_ALIGNMENT, is specified, then we would
treat it as an error.
So that we can detect unexpected chunks from btrfs-progs, and fix them
before reaching the end users.

Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 check/common.h      |  1 +
 check/main.c        | 20 ++++++++++++++++++++
 check/mode-lowmem.c | 11 +++++++++++
 common/utils.c      | 19 +++++++++++++++++++
 common/utils.h      |  1 +
 5 files changed, 52 insertions(+)
diff mbox series

Patch

diff --git a/check/common.h b/check/common.h
index 2d5db2131ba5..d1c8e8de4af9 100644
--- a/check/common.h
+++ b/check/common.h
@@ -78,6 +78,7 @@  struct chunk_record {
 	u32 io_align;
 	u32 io_width;
 	u32 sector_size;
+	bool unaligned;
 	struct stripe stripes[0];
 };
 
diff --git a/check/main.c b/check/main.c
index 108818688132..99643f0ad7e0 100644
--- a/check/main.c
+++ b/check/main.c
@@ -5265,6 +5265,9 @@  struct chunk_record *btrfs_new_chunk_record(struct extent_buffer *leaf,
 	rec->num_stripes = num_stripes;
 	rec->sub_stripes = btrfs_chunk_sub_stripes(leaf, ptr);
 
+	if (!IS_ALIGNED(rec->cache.start, BTRFS_STRIPE_LEN)||
+	    !IS_ALIGNED(rec->cache.size, BTRFS_STRIPE_LEN))
+		rec->unaligned = true;
 	for (i = 0; i < rec->num_stripes; ++i) {
 		rec->stripes[i].devid =
 			btrfs_stripe_devid_nr(leaf, ptr, i);
@@ -8386,6 +8389,7 @@  int check_chunks(struct cache_tree *chunk_cache,
 	struct chunk_record *chunk_rec;
 	struct block_group_record *bg_rec;
 	struct device_extent_record *dext_rec;
+	bool strict_alignment = get_env_bool("BTRFS_DEBUG_STRICT_CHUNK_ALIGNMENT");
 	int err;
 	int ret = 0;
 
@@ -8393,6 +8397,22 @@  int check_chunks(struct cache_tree *chunk_cache,
 	while (chunk_item) {
 		chunk_rec = container_of(chunk_item, struct chunk_record,
 					 cache);
+		if (chunk_rec->unaligned && !silent) {
+			if (strict_alignment) {
+				error(
+		"chunk[%llu %llu) is not fully aligned to BTRFS_STRIPE_LEN(%u)",
+				      chunk_rec->cache.start,
+				      chunk_rec->cache.start + chunk_rec->cache.size,
+				      BTRFS_STRIPE_LEN);
+				ret = -EINVAL;
+			} else {
+				warning(
+		"chunk[%llu %llu) is not fully aligned to BTRFS_STRIPE_LEN(%u)",
+				      chunk_rec->cache.start,
+				      chunk_rec->cache.start + chunk_rec->cache.size,
+				      BTRFS_STRIPE_LEN);
+			}
+		}
 		err = check_chunk_refs(chunk_rec, block_group_cache,
 				       dev_extent_cache, silent);
 		if (err < 0)
diff --git a/check/mode-lowmem.c b/check/mode-lowmem.c
index 19b7b1a72a1f..39944c5430ec 100644
--- a/check/mode-lowmem.c
+++ b/check/mode-lowmem.c
@@ -4719,6 +4719,17 @@  static int check_chunk_item(struct extent_buffer *eb, int slot)
 	chunk = btrfs_item_ptr(eb, slot, struct btrfs_chunk);
 	length = btrfs_chunk_length(eb, chunk);
 	chunk_end = chunk_key.offset + length;
+	if (!IS_ALIGNED(chunk_key.offset, BTRFS_STRIPE_LEN) ||
+	    !IS_ALIGNED(length, BTRFS_STRIPE_LEN)) {
+		if (get_env_bool("BTRFS_PROGS_DEBUG_STRICT_CHUNK_ALIGNMENT")) {
+			error("chunk[%llu %llu) is not fully aligned to BTRFS_STRIPE_LEN(%u)",
+				chunk_key.offset, length, BTRFS_STRIPE_LEN);
+			err |= BYTES_UNALIGNED;
+			goto out;
+		}
+		warning("chunk[%llu %llu) is not fully aligned to BTRFS_STRIPE_LEN(%u)",
+			chunk_key.offset, length, BTRFS_STRIPE_LEN);
+	}
 	ret = btrfs_check_chunk_valid(eb, chunk, chunk_key.offset);
 	if (ret < 0) {
 		error("chunk[%llu %llu) is invalid", chunk_key.offset,
diff --git a/common/utils.c b/common/utils.c
index e6070791f5cc..68fa95ece6f8 100644
--- a/common/utils.c
+++ b/common/utils.c
@@ -1018,6 +1018,25 @@  int get_env_u64(const char *env_name, u64 *value_ret)
 	return 0;
 }
 
+/*
+ * Parse a boolean value from an environment variable.
+ *
+ * As long as the environment variable is not set to "0", "n" or "\0",
+ * it would return true.
+ */
+bool get_env_bool(const char *env_name)
+{
+	char *env_value_str;
+
+	env_value_str = getenv(env_name);
+	if (!env_value_str)
+		return false;
+	if (env_value_str[0] == '0' || env_value_str[0] == 'n' ||
+	    env_value_str[0] == '\0')
+		return false;
+	return true;
+}
+
 void btrfs_config_init(void)
 {
 	bconf.output_format = CMD_FORMAT_TEXT;
diff --git a/common/utils.h b/common/utils.h
index 30c75339b05b..5bdeeab44bb4 100644
--- a/common/utils.h
+++ b/common/utils.h
@@ -133,6 +133,7 @@  u64 rand_u64(void);
 unsigned int rand_range(unsigned int upper);
 void init_rand_seed(u64 seed);
 int get_env_u64(const char *env_name, u64 *value_ret);
+bool get_env_bool(const char *env_name);
 
 char *btrfs_test_for_multiple_profiles(int fd);
 int btrfs_warn_multiple_profiles(int fd);