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.