* [PATCH 2/3] btrfs-progs: add extra chunk alignment checks
2024-01-16 3:31 [PATCH 0/3] btrfs-progs: make convert to generate chunks aligned to stripe boundary Qu Wenruo
2024-01-16 3:31 ` [PATCH 1/3] btrfs-progs: convert: make sure the length of data chunks are also stripe aligned Qu Wenruo
@ 2024-01-16 3:31 ` Qu Wenruo
2024-01-16 3:31 ` [PATCH 3/3] btrfs-progs: tests: enable strict chunk alignment check Qu Wenruo
2024-01-16 18:16 ` [PATCH 0/3] btrfs-progs: make convert to generate chunks aligned to stripe boundary David Sterba
3 siblings, 0 replies; 5+ messages in thread
From: Qu Wenruo @ 2024-01-16 3:31 UTC (permalink / raw)
To: linux-btrfs
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 --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);
--
2.43.0
^ permalink raw reply related [flat|nested] 5+ messages in thread* Re: [PATCH 0/3] btrfs-progs: make convert to generate chunks aligned to stripe boundary
2024-01-16 3:31 [PATCH 0/3] btrfs-progs: make convert to generate chunks aligned to stripe boundary Qu Wenruo
` (2 preceding siblings ...)
2024-01-16 3:31 ` [PATCH 3/3] btrfs-progs: tests: enable strict chunk alignment check Qu Wenruo
@ 2024-01-16 18:16 ` David Sterba
3 siblings, 0 replies; 5+ messages in thread
From: David Sterba @ 2024-01-16 18:16 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs
On Tue, Jan 16, 2024 at 02:01:23PM +1030, Qu Wenruo wrote:
> There is a recent report about scrub use-after-free, which is caused by
> unaligned chunk length (only aligned to sectorsize, but not to
> BTRFS_STRIPE_LEN).
>
> Although the bug would soon be fixed in kernel, there is no hard to make
> convert to generate data chunks with both start and length aligned to
> BTRFS_STRIPE_LEN.
>
> Thankfully the start bytenr is already aligned to 64K, we only need to
> make the length aligned.
>
> Furthermore, allow "btrfs check" to detect such unaligned chunks and
> gives a warning (but not consider it as an error).
> For selftests, we would utilize the debug environment variable,
> BTRFS_PROGS_DEBUG_STRICT_CHUNK_ALIGNMENT, to convert the warning to an
> error.
>
> Qu Wenruo (3):
> btrfs-progs: convert: make sure the length of data chunks are also
> stripe aligned
> btrfs-progs: add extra chunk alignment checks
> btrfs-progs: tests: enable strict chunk alignment check
Added to devel, with some minor fixups, thanks. The environment variable
name is long but it's just for us and it's descriptive, so OK.
^ permalink raw reply [flat|nested] 5+ messages in thread