Re: [PATCH v2 1/2] dm array: validate array block headers on read

Ming Hung Tsai <[email protected]> Mon, 3 Aug 2026 21:41:36 +0800
Newsgroups dev.linux.lists.dm-devel,org.kernel.vger.linux-kernel
Message-ID <CALjSBEvVOTpd+j3zgspvdbX2fhQwc1HoNQHFup0uEfkRM=2kVQ@mail.gmail.com>
On Sat, Aug 1, 2026 at 6:55=E2=80=AFAM Bryam Vargas via B4 Relay
<[email protected]> wrote:
>
> From: Bryam Vargas <[email protected]>
>
> 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 <[email protected]>
> Cc: [email protected]
> Signed-off-by: Bryam Vargas <[email protected]>

Reviewed-by: Ming-Hung Tsai <[email protected]>

Reproducer to validate the BUG_ON prevention using crafted metadata:
create a cache device, and reduce the max_entries of the final array
block of the mapping array. Next, expand the fast device to make the
last array block exceeds its capacity, which triggers the BUG_ON in
fill_ablock().

> +       wanted =3D calc_max_entries(value_size, size_of_block);
> +       if (max_entries !=3D wanted) {
> +               DMERR_LIMIT("%s failed: max_entries %u !=3D wanted %u for=
 value_size %u",
> +                           __func__, max_entries, wanted, value_size);
> +               return -EILSEQ;
> +       }

One minor nit, which needn't hold up the patch: I'd drop "wanted %u"
from the error message. The wanted value is derivable from value_size,
so printing "%s failed: max_entries %u invalid for value_size %u"
carries the same information.

Mikulas, if you agree, feel free to adjust when applying rather than
requesting a v3 patch. Thank you.