* [PATCH 6.18 2/4] dm-verity-fec: fix reading parity bytes split across blocks (take 3)
2026-07-29 5:19 [PATCH 6.18 0/4] dm-verity fixes Eric Biggers
2026-07-29 5:19 ` [PATCH 6.18 1/4] dm-verity-fec: fix the size of dm_verity_fec_io::erasures Eric Biggers
@ 2026-07-29 5:19 ` Eric Biggers
2026-07-29 5:19 ` [PATCH 6.18 3/4] dm-verity-fec: replace {MAX,MIN}_RSN with {MIN,MAX}_ROOTS Eric Biggers
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2026-07-29 5:19 UTC (permalink / raw)
To: stable; +Cc: dm-devel, Mikulas Patocka, Eric Biggers
commit 430a05cb926f6bdf53e81460a2c3a553257f3f61 upstream.
fec_decode_bufs() assumes that the parity bytes of the first RS codeword
it decodes are never split across parity blocks.
This assumption is false. Consider v->fec->block_size == 4096 &&
v->fec->roots == 17 && fio->nbufs == 1, for example. In that case, each
call to fec_decode_bufs() consumes v->fec->roots * (fio->nbufs <<
DM_VERITY_FEC_BUF_RS_BITS) = 272 parity bytes.
Considering that the parity data for each message block starts on a
block boundary, the byte alignment in the parity data will iterate
through 272*i mod 4096 until the 3 parity blocks have been consumed. On
the 16th call (i=15), the alignment will be 4080 bytes into the first
block. Only 16 bytes remain in that block, but 17 parity bytes will be
needed. The code reads out-of-bounds from the parity block buffer.
Fortunately this doesn't normally happen, since it can occur only for
certain non-default values of fec_roots *and* when the maximum number of
buffers couldn't be allocated due to low memory. For example with
block_size=4096 only the following cases are affected:
fec_roots=17: nbufs in [1, 3, 5, 15]
fec_roots=19: nbufs in [1, 229]
fec_roots=21: nbufs in [1, 3, 5, 13, 15, 39, 65, 195]
fec_roots=23: nbufs in [1, 89]
Regardless, fix it by refactoring how the parity blocks are read.
Fixes: 6df90c02bae4 ("dm-verity FEC: Fix RS FEC repair for roots unaligned to block size (take 2)")
Cc: stable@vger.kernel.org
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
drivers/md/dm-verity-fec.c | 100 ++++++++++++++++---------------------
1 file changed, 44 insertions(+), 56 deletions(-)
diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index 03e59b85132aa..a8fbf97b97517 100644
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -39,36 +39,6 @@ static inline u64 fec_interleave(struct dm_verity *v, u64 offset)
return offset + mod * (v->fec->rounds << v->data_dev_block_bits);
}
-/*
- * Read error-correcting codes for the requested RS block. Returns a pointer
- * to the data block. Caller is responsible for releasing buf.
- */
-static u8 *fec_read_parity(struct dm_verity *v, u64 rsb, int index,
- unsigned int *offset, unsigned int par_buf_offset,
- struct dm_buffer **buf, unsigned short ioprio)
-{
- u64 position, block, rem;
- u8 *res;
-
- /* We have already part of parity bytes read, skip to the next block */
- if (par_buf_offset)
- index++;
-
- position = (index + rsb) * v->fec->roots;
- block = div64_u64_rem(position, v->fec->io_size, &rem);
- *offset = par_buf_offset ? 0 : (unsigned int)rem;
-
- res = dm_bufio_read_with_ioprio(v->fec->bufio, block, buf, ioprio);
- if (IS_ERR(res)) {
- DMERR("%s: FEC %llu: parity read failed (block %llu): %ld",
- v->data_dev->name, (unsigned long long)rsb,
- (unsigned long long)block, PTR_ERR(res));
- *buf = NULL;
- }
-
- return res;
-}
-
/* Loop over each preallocated buffer slot. */
#define fec_for_each_prealloc_buffer(__i) \
for (__i = 0; __i < DM_VERITY_FEC_BUF_PREALLOC; __i++)
@@ -116,15 +86,29 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_io *io,
{
int r, corrected = 0, res;
struct dm_buffer *buf;
- unsigned int n, i, j, offset, par_buf_offset = 0;
+ unsigned int n, i, j, parity_pos, to_copy;
uint16_t par_buf[DM_VERITY_FEC_RSM - DM_VERITY_FEC_MIN_RSN];
u8 *par, *block;
+ u64 parity_block;
struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
- par = fec_read_parity(v, rsb, block_offset, &offset,
- par_buf_offset, &buf, bio->bi_ioprio);
- if (IS_ERR(par))
+ /*
+ * Compute the index of the first parity block that will be needed and
+ * the starting position in that block. Then read that block.
+ *
+ * io_size is always a power of 2, but roots might not be. Note that
+ * when it's not, a codeword's parity bytes can span a block boundary.
+ */
+ parity_block = (rsb + block_offset) * v->fec->roots;
+ parity_pos = parity_block & (v->fec->io_size - 1);
+ parity_block >>= v->data_dev_block_bits;
+ par = dm_bufio_read_with_ioprio(v->fec->bufio, parity_block, &buf,
+ bio->bi_ioprio);
+ if (IS_ERR(par)) {
+ DMERR("%s: FEC %llu: parity read failed (block %llu): %ld",
+ v->data_dev->name, rsb, parity_block, PTR_ERR(par));
return PTR_ERR(par);
+ }
/*
* Decode the RS blocks we have in bufs. Each RS block results in
@@ -132,8 +116,32 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_io *io,
*/
fec_for_each_buffer_rs_block(fio, n, i) {
block = fec_buffer_rs_block(v, fio, n, i);
- for (j = 0; j < v->fec->roots - par_buf_offset; j++)
- par_buf[par_buf_offset + j] = par[offset + j];
+
+ /*
+ * Copy the next 'roots' parity bytes to 'par_buf', reading
+ * another parity block if needed.
+ */
+ to_copy = min(v->fec->io_size - parity_pos, v->fec->roots);
+ for (j = 0; j < to_copy; j++)
+ par_buf[j] = par[parity_pos++];
+ if (to_copy < v->fec->roots) {
+ parity_block++;
+ parity_pos = 0;
+
+ dm_bufio_release(buf);
+ par = dm_bufio_read_with_ioprio(v->fec->bufio,
+ parity_block, &buf,
+ bio->bi_ioprio);
+ if (IS_ERR(par)) {
+ DMERR("%s: FEC %llu: parity read failed (block %llu): %ld",
+ v->data_dev->name, rsb, parity_block,
+ PTR_ERR(par));
+ return PTR_ERR(par);
+ }
+ for (; j < v->fec->roots; j++)
+ par_buf[j] = par[parity_pos++];
+ }
+
/* Decode an RS block using Reed-Solomon */
res = decode_rs8(fio->rs, block, par_buf, v->fec->rsn,
NULL, neras, fio->erasures, 0, NULL);
@@ -148,26 +156,6 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_io *io,
block_offset++;
if (block_offset >= 1 << v->data_dev_block_bits)
goto done;
-
- /* Read the next block when we run out of parity bytes */
- offset += (v->fec->roots - par_buf_offset);
- /* Check if parity bytes are split between blocks */
- if (offset < v->fec->io_size && (offset + v->fec->roots) > v->fec->io_size) {
- par_buf_offset = v->fec->io_size - offset;
- for (j = 0; j < par_buf_offset; j++)
- par_buf[j] = par[offset + j];
- offset += par_buf_offset;
- } else
- par_buf_offset = 0;
-
- if (offset >= v->fec->io_size) {
- dm_bufio_release(buf);
-
- par = fec_read_parity(v, rsb, block_offset, &offset,
- par_buf_offset, &buf, bio->bi_ioprio);
- if (IS_ERR(par))
- return PTR_ERR(par);
- }
}
done:
r = corrected;
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6.18 3/4] dm-verity-fec: replace {MAX,MIN}_RSN with {MIN,MAX}_ROOTS
2026-07-29 5:19 [PATCH 6.18 0/4] dm-verity fixes Eric Biggers
2026-07-29 5:19 ` [PATCH 6.18 1/4] dm-verity-fec: fix the size of dm_verity_fec_io::erasures Eric Biggers
2026-07-29 5:19 ` [PATCH 6.18 2/4] dm-verity-fec: fix reading parity bytes split across blocks (take 3) Eric Biggers
@ 2026-07-29 5:19 ` Eric Biggers
2026-07-29 5:19 ` [PATCH 6.18 4/4] dm-verity: fix buffer overflow in FEC calculation Eric Biggers
2026-07-30 2:40 ` [PATCH 6.18 0/4] dm-verity fixes Sasha Levin
4 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2026-07-29 5:19 UTC (permalink / raw)
To: stable; +Cc: dm-devel, Mikulas Patocka, Eric Biggers
commit 82fbd6a3e29a329d439690cd7ccc4162c9cd8db6 upstream.
Every time DM_VERITY_FEC_{MAX,MIN}_RSN are used, they are subtracted
from DM_VERITY_FEC_RSM to get the bounds on the number of roots.
Therefore, replace these with {MIN,MAX}_ROOTS constants which are more
directly useful. (Note the inversion, where MAX_RSN maps to MIN_ROOTS
and MIN_RSN maps to MAX_ROOTS.) No functional change.
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
---
drivers/md/dm-verity-fec.c | 6 +++---
drivers/md/dm-verity-fec.h | 7 +++----
2 files changed, 6 insertions(+), 7 deletions(-)
diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index a8fbf97b97517..cebcc8fd25d70 100644
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -87,7 +87,7 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_io *io,
int r, corrected = 0, res;
struct dm_buffer *buf;
unsigned int n, i, j, parity_pos, to_copy;
- uint16_t par_buf[DM_VERITY_FEC_RSM - DM_VERITY_FEC_MIN_RSN];
+ uint16_t par_buf[DM_VERITY_FEC_MAX_ROOTS];
u8 *par, *block;
u64 parity_block;
struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
@@ -605,8 +605,8 @@ int verity_fec_parse_opt_args(struct dm_arg_set *as, struct dm_verity *v,
} else if (!strcasecmp(arg_name, DM_VERITY_OPT_FEC_ROOTS)) {
if (sscanf(arg_value, "%hhu%c", &num_c, &dummy) != 1 || !num_c ||
- num_c < (DM_VERITY_FEC_RSM - DM_VERITY_FEC_MAX_RSN) ||
- num_c > (DM_VERITY_FEC_RSM - DM_VERITY_FEC_MIN_RSN)) {
+ num_c < DM_VERITY_FEC_MIN_ROOTS ||
+ num_c > DM_VERITY_FEC_MAX_ROOTS) {
ti->error = "Invalid " DM_VERITY_OPT_FEC_ROOTS;
return -EINVAL;
}
diff --git a/drivers/md/dm-verity-fec.h b/drivers/md/dm-verity-fec.h
index 90a0af3f35d31..b3460103e0e10 100644
--- a/drivers/md/dm-verity-fec.h
+++ b/drivers/md/dm-verity-fec.h
@@ -13,8 +13,8 @@
/* Reed-Solomon(M, N) parameters */
#define DM_VERITY_FEC_RSM 255
-#define DM_VERITY_FEC_MAX_RSN 253
-#define DM_VERITY_FEC_MIN_RSN 231 /* ~10% space overhead */
+#define DM_VERITY_FEC_MIN_ROOTS 2 /* RS(255, 253): ~0.8% space overhead */
+#define DM_VERITY_FEC_MAX_ROOTS 24 /* RS(255, 231): ~10% space overhead */
/* buffers for deinterleaving and decoding */
#define DM_VERITY_FEC_BUF_PREALLOC 1 /* buffers to preallocate */
@@ -50,8 +50,7 @@ struct dm_verity_fec {
/* per-bio data */
struct dm_verity_fec_io {
struct rs_control *rs; /* Reed-Solomon state */
- /* erasures for decode_rs8 */
- int erasures[DM_VERITY_FEC_RSM - DM_VERITY_FEC_MIN_RSN + 1];
+ int erasures[DM_VERITY_FEC_MAX_ROOTS + 1]; /* erasures for decode_rs8 */
u8 *bufs[DM_VERITY_FEC_BUF_MAX]; /* bufs for deinterleaving */
unsigned int nbufs; /* number of buffers allocated */
u8 *output; /* buffer for corrected output */
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread* [PATCH 6.18 4/4] dm-verity: fix buffer overflow in FEC calculation
2026-07-29 5:19 [PATCH 6.18 0/4] dm-verity fixes Eric Biggers
` (2 preceding siblings ...)
2026-07-29 5:19 ` [PATCH 6.18 3/4] dm-verity-fec: replace {MAX,MIN}_RSN with {MIN,MAX}_ROOTS Eric Biggers
@ 2026-07-29 5:19 ` Eric Biggers
2026-07-30 2:40 ` [PATCH 6.18 0/4] dm-verity fixes Sasha Levin
4 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2026-07-29 5:19 UTC (permalink / raw)
To: stable; +Cc: dm-devel, Mikulas Patocka, Sami Tolvanen, Eric Biggers
From: Mikulas Patocka <mpatocka@redhat.com>
commit 31d6e6c0ba8d5a7bd59660035a089307100c5e8e upstream.
There's a buffer overflow in dm-verity-fec:
if (neras && *neras <= v->fec->roots)
fio->erasures[(*neras)++] = i;
This allows *neras to reach roots + 1 (the post-increment pushes it past
roots). This value is then passed as no_eras to decode_rs8(). Inside the
RS decoder (lib/reed_solomon/decode_rs.c:113-121), the erasure locator
polynomial loop writes lambda[j] where j can reach nroots + 1 — one
element past the end of lambda[] (which is sized nroots + 1, valid
indices 0..nroots). The out-of-bounds write lands on syn[0], corrupting
the syndrome buffer.
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Assisted-by: Claude:claude-opus-4-6
Cc: stable@vger.kernel.org
Fixes: a739ff3f543a ("dm verity: add support for forward error correction")
Reviewed-by: Sami Tolvanen <samitolvanen@google.com>
Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---
drivers/md/dm-verity-fec.c | 4 ++--
drivers/md/dm-verity-fec.h | 2 +-
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index cebcc8fd25d70..d1e4fbc5a21d9 100644
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -250,7 +250,7 @@ static int fec_read_bufs(struct dm_verity *v, struct dm_verity_io *io,
(unsigned long long)block, PTR_ERR(bbuf));
/* assume the block is corrupted */
- if (neras && *neras <= v->fec->roots)
+ if (neras && *neras < v->fec->roots)
fio->erasures[(*neras)++] = i;
continue;
@@ -268,7 +268,7 @@ static int fec_read_bufs(struct dm_verity *v, struct dm_verity_io *io,
* skip if we have already found the theoretical
* maximum number (i.e. fec->roots) of erasures
*/
- if (neras && *neras <= v->fec->roots &&
+ if (neras && *neras < v->fec->roots &&
fec_is_erasure(v, io, want_digest, bbuf))
fio->erasures[(*neras)++] = i;
}
diff --git a/drivers/md/dm-verity-fec.h b/drivers/md/dm-verity-fec.h
index b3460103e0e10..8552b5d3c9152 100644
--- a/drivers/md/dm-verity-fec.h
+++ b/drivers/md/dm-verity-fec.h
@@ -50,7 +50,7 @@ struct dm_verity_fec {
/* per-bio data */
struct dm_verity_fec_io {
struct rs_control *rs; /* Reed-Solomon state */
- int erasures[DM_VERITY_FEC_MAX_ROOTS + 1]; /* erasures for decode_rs8 */
+ int erasures[DM_VERITY_FEC_MAX_ROOTS]; /* erasures for decode_rs8 */
u8 *bufs[DM_VERITY_FEC_BUF_MAX]; /* bufs for deinterleaving */
unsigned int nbufs; /* number of buffers allocated */
u8 *output; /* buffer for corrected output */
--
2.55.0
^ permalink raw reply related [flat|nested] 6+ messages in thread