All of lore.kernel.org
 help / color / mirror / Atom feed
* Patch "dm array: validate array block headers on read" has been added to the 6.6-stable tree
@ 2026-10-16 18:02 Sasha Levin
  2026-10-22 16:35 ` sashiko-bot
  0 siblings, 1 reply; 2+ messages in thread
From: Sasha Levin @ 2026-10-16 18:02 UTC (permalink / raw)
  To: stable-commits, hexlabsecurity; +Cc: Alasdair Kergon, Mike Snitzer, dm-devel

This is a note to let you know that I've just added the patch titled

    dm array: validate array block headers on read

to the 6.6-stable tree which can be found at:
    http://www.kernel.org/git/?p=linux/kernel/git/stable/stable-queue.git;a=summary

The filename of the patch is:
     dm-array-validate-array-block-headers-on-read.patch
and it can be found in the queue-6.6 subdirectory.

If you, or anyone else, feels it should not be added to the stable tree,
please let <stable@vger.kernel.org> know about it.



commit f12dd69353008d76c1914e34993f033fb23bca89
Author: Bryam Vargas <hexlabsecurity@proton.me>
Date:   Fri Jul 31 17:54:54 2026 -0500

    dm array: validate array block headers on read
    
    [ Upstream commit 2965787723084835b18dfe993cd450ebf5bd4540 ]
    
    array_block_check() validates blocknr and csum and nothing else, while
    node_check(), next to it, has bounded the structural fields since both
    were written. dm_array_cursor_next() takes its loop bound from the
    on-disk nr_entries and element_at() is unguarded pointer arithmetic, so
    a count larger than the block holds keeps the cursor in one block while
    the index grows past it and the read walks off the dm-bufio buffer --
    dm_cache_load_mappings() drives it once per cache block at activation.
    
    Check the header against itself: reject a zero value_size, require
    max_entries to equal calc_max_entries() for that value_size and block
    size, and require nr_entries to fit. Equality rather than an upper bound,
    since a count below the real capacity trips BUG_ON() in fill_ablock() and
    trim_ablock(). Metadata dm-array writes satisfies all three.
    
    Fixes: 6513c29f44f2 ("dm persistent data: add transactional array")
    Suggested-by: Ming-Hung Tsai <mtsai@redhat.com>
    Cc: stable@vger.kernel.org
    Signed-off-by: Bryam Vargas <hexlabsecurity@proton.me>
    Reviewed-by: Ming-Hung Tsai <mtsai@redhat.com>
    Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
    Signed-off-by: Sasha Levin <sashal@kernel.org>

diff --git a/drivers/md/persistent-data/dm-array.c b/drivers/md/persistent-data/dm-array.c
index 5e9c17307671c..1969d6cfbfa2b 100644
--- a/drivers/md/persistent-data/dm-array.c
+++ b/drivers/md/persistent-data/dm-array.c
@@ -38,6 +38,14 @@ struct array_block {
  */
 #define CSUM_XOR 595846735
 
+/*
+ * Each array block can hold this many values.
+ */
+static uint32_t calc_max_entries(size_t value_size, size_t size_of_block)
+{
+	return (size_of_block - sizeof(struct array_block)) / value_size;
+}
+
 static void array_block_prepare_for_write(struct dm_block_validator *v,
 					  struct dm_block *b,
 					  size_t size_of_block)
@@ -55,6 +63,7 @@ static int array_block_check(struct dm_block_validator *v,
 			     size_t size_of_block)
 {
 	struct array_block *bh_le = dm_block_data(b);
+	uint32_t nr_entries, max_entries, value_size;
 	__le32 csum_disk;
 
 	if (dm_block_location(b) != le64_to_cpu(bh_le->blocknr)) {
@@ -74,6 +83,26 @@ static int array_block_check(struct dm_block_validator *v,
 		return -EILSEQ;
 	}
 
+	nr_entries = le32_to_cpu(bh_le->nr_entries);
+	max_entries = le32_to_cpu(bh_le->max_entries);
+	value_size = le32_to_cpu(bh_le->value_size);
+
+	if (!value_size) {
+		DMERR_LIMIT("%s failed: value_size is zero", __func__);
+		return -EILSEQ;
+	}
+
+	if (max_entries != calc_max_entries(value_size, size_of_block)) {
+		DMERR_LIMIT("%s failed: max_entries %u invalid for value_size %u",
+			    __func__, max_entries, value_size);
+		return -EILSEQ;
+	}
+
+	if (nr_entries > max_entries) {
+		DMERR_LIMIT("%s failed: too many entries", __func__);
+		return -EILSEQ;
+	}
+
 	return 0;
 }
 
@@ -138,14 +167,6 @@ static void dec_ablock_entries(struct dm_array_info *info, struct array_block *a
 		on_entries(info, ab, vt->dec);
 }
 
-/*
- * Each array block can hold this many values.
- */
-static uint32_t calc_max_entries(size_t value_size, size_t size_of_block)
-{
-	return (size_of_block - sizeof(struct array_block)) / value_size;
-}
-
 /*
  * Allocate a new array block.  The caller will need to unlock block.
  */

^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: Patch "dm array: validate array block headers on read" has been added to the 6.6-stable tree
  2026-10-16 18:02 Patch "dm array: validate array block headers on read" has been added to the 6.6-stable tree Sasha Levin
@ 2026-10-22 16:35 ` sashiko-bot
  0 siblings, 0 replies; 2+ messages in thread
From: sashiko-bot @ 2026-10-22 16:35 UTC (permalink / raw)
  To: Sasha Levin, hexlabsecurity; +Cc: Alasdair Kergon, Mike Snitzer, dm-devel

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] dm array: array_block_check() fails to validate value_size against info
--

commit 3c810b656c43751de89392b584e21daeb9a84fcd
Author: Sasha Levin <sashal@kernel.org>

Patch "dm array: validate array block headers on read" has been added to the 6.6-stable tree

This commit backports upstream commit 296578772308 to validate on-disk
array_block headers in array_block_check() by checking that value_size is
non-zero, max_entries matches calc_max_entries(), and nr_entries does not
exceed max_entries.

> diff --git a/drivers/md/persistent-data/dm-array.c b/drivers/md/persistent-data/dm-array.c
> index 798c9c53a3435..fb65e82dab890 100644
> --- a/drivers/md/persistent-data/dm-array.c
> +++ b/drivers/md/persistent-data/dm-array.c
[ ... ]
> @@ -74,6 +83,26 @@ static int array_block_check(struct dm_block_validator *v,
>  		return -EILSEQ;
>  	}
>  
> +	nr_entries = le32_to_cpu(bh_le->nr_entries);
> +	max_entries = le32_to_cpu(bh_le->max_entries);
> +	value_size = le32_to_cpu(bh_le->value_size);
> +
> +	if (!value_size) {
> +		DMERR_LIMIT("%s failed: value_size is zero", __func__);
> +		return -EILSEQ;
> +	}
> +
> +	if (max_entries != calc_max_entries(value_size, size_of_block)) {
> +		DMERR_LIMIT("%s failed: max_entries %u invalid for value_size %u",
> +			    __func__, max_entries, value_size);
> +		return -EILSEQ;
> +	}
> +
> +	if (nr_entries > max_entries) {
> +		DMERR_LIMIT("%s failed: too many entries", __func__);
> +		return -EILSEQ;
> +	}
> +
>  	return 0;
>  }

[Severity: High]
Does validating max_entries and nr_entries only against the on-disk
bh_le->value_size in array_block_check() still allow out-of-bounds reads
in element_at() and trigger the BUG_ON() checks in fill_ablock() and
trim_ablock() when bh_le->value_size does not match info->value_type.size?

In drivers/md/persistent-data/dm-array.c, neither get_ablock() nor
__shadow_ablock() checks that the on-disk ab->value_size matches
info->value_type.size after dm_tm_read_lock() or dm_tm_shadow_block()
validates the block:

drivers/md/persistent-data/dm-array.c:get_ablock() {
	r = dm_tm_read_lock(info->btree_info.tm, b, &array_validator, block);
	if (r)
		return r;

	*ab = dm_block_data(*block);
	return 0;
}

However, element_at() computes entry offsets using info->value_type.size
rather than ab->value_size:

drivers/md/persistent-data/dm-array.c:element_at() {
	unsigned char *entry = (unsigned char *) (ab + 1);

	entry += index * info->value_type.size;

	return entry;
}

If a corrupted or crafted on-disk array_block header has a smaller
value_size than info->value_type.size (for example, value_size = 1,
max_entries = 4072, and nr_entries = 4072 in a 4096-byte block when
info->value_type.size is 8 in dm-cache or dm-bitset), array_block_check()
succeeds. Callers that iterate up to le32_to_cpu(ab->nr_entries), such as
dm_array_cursor_next() and dm_array_cursor_get_value() (via
dm_cache_load_mappings()), walk_ablock(), on_entries() (via
inc_ablock_entries() and dec_ablock_entries()), and trim_ablock(), will
still advance index up to 4071 while element_at() strides by
info->value_type.size, reading past the end of the dm-bufio block buffer.

Conversely, if on-disk bh_le->value_size is larger than
info->value_type.size, array_block_check() accepts a smaller
ab->max_entries than the capacity computed from info->value_type.size in
dm_array_resize() -> array_resize(). When array_resize() passes new_nr up
to resize->max_entries into fill_ablock() or trim_ablock(), it can still
trigger BUG_ON(new_nr > le32_to_cpu(ab->max_entries)):

drivers/md/persistent-data/dm-array.c:fill_ablock() {
	uint32_t nr_entries, delta, i;
	struct dm_btree_value_type *vt = &info->value_type;

	BUG_ON(new_nr > le32_to_cpu(ab->max_entries));
	BUG_ON(new_nr < le32_to_cpu(ab->nr_entries));
	...
}

Should get_ablock() and __shadow_ablock() also verify that
le32_to_cpu((*ab)->value_size) == info->value_type.size before using or
incrementing the block entries?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261016180222.4058164-1-sashal@kernel.org?part=1

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-10-22 16:35 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-10-16 18:02 Patch "dm array: validate array block headers on read" has been added to the 6.6-stable tree Sasha Levin
2026-10-22 16:35 ` sashiko-bot

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.