Skip to content

Commit 6df90c0

Browse files
mbrozMikulas Patocka
authored andcommitted
dm-verity FEC: Fix RS FEC repair for roots unaligned to block size (take 2)
This patch fixes an issue that was fixed in the commit df7b59b ("dm verity: fix FEC for RS roots unaligned to block size") but later broken again in the commit 8ca7cab ("dm verity fec: fix misaligned RS roots IO") If the Reed-Solomon roots setting spans multiple blocks, the code does not use proper parity bytes and randomly fails to repair even trivial errors. This bug cannot happen if the sector size is multiple of RS roots setting (Android case with roots 2). The previous solution was to find a dm-bufio block size that is multiple of the device sector size and roots size. Unfortunately, the optimization in commit 8ca7cab ("dm verity fec: fix misaligned RS roots IO") is incorrect and uses data block size for some roots (for example, it uses 4096 block size for roots = 20). This patch uses a different approach: - It always uses a configured data block size for dm-bufio to avoid possible misaligned IOs. - and it caches the processed parity bytes, so it can join it if it spans two blocks. As the RS calculation is called only if an error is detected and the process is computationally intensive, copying a few more bytes should not introduce performance issues. The issue was reported to cryptsetup with trivial reproducer https://gitlab.com/cryptsetup/cryptsetup/-/issues/923 Reproducer (with roots=20): # create verity device with RS FEC dd if=/dev/urandom of=data.img bs=4096 count=8 status=none veritysetup format data.img hash.img --fec-device=fec.img --fec-roots=20 | \ awk '/^Root hash/{ print $3 }' >roothash # create an erasure that should always be repairable with this roots setting dd if=/dev/zero of=data.img conv=notrunc bs=1 count=4 seek=4 status=none # try to read it through dm-verity veritysetup open data.img test hash.img --fec-device=fec.img --fec-roots=20 $(cat roothash) dd if=/dev/mapper/test of=/dev/null bs=4096 status=noxfer Even now the log says it cannot repair it: : verity-fec: 7:1: FEC 0: failed to correct: -74 : device-mapper: verity: 7:1: data block 0 is corrupted ... With this fix, errors are properly repaired. : verity-fec: 7:1: FEC 0: corrected 4 errors Signed-off-by: Milan Broz <gmazyland@gmail.com> Fixes: 8ca7cab ("dm verity fec: fix misaligned RS roots IO") Cc: stable@vger.kernel.org Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
1 parent 0bb1968 commit 6df90c0

File tree

1 file changed

+26
-14
lines changed

1 file changed

+26
-14
lines changed

drivers/md/dm-verity-fec.c

Lines changed: 26 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -60,15 +60,19 @@ static int fec_decode_rs8(struct dm_verity *v, struct dm_verity_fec_io *fio,
6060
* to the data block. Caller is responsible for releasing buf.
6161
*/
6262
static u8 *fec_read_parity(struct dm_verity *v, u64 rsb, int index,
63-
unsigned int *offset, struct dm_buffer **buf,
64-
unsigned short ioprio)
63+
unsigned int *offset, unsigned int par_buf_offset,
64+
struct dm_buffer **buf, unsigned short ioprio)
6565
{
6666
u64 position, block, rem;
6767
u8 *res;
6868

69+
/* We have already part of parity bytes read, skip to the next block */
70+
if (par_buf_offset)
71+
index++;
72+
6973
position = (index + rsb) * v->fec->roots;
7074
block = div64_u64_rem(position, v->fec->io_size, &rem);
71-
*offset = (unsigned int)rem;
75+
*offset = par_buf_offset ? 0 : (unsigned int)rem;
7276

7377
res = dm_bufio_read_with_ioprio(v->fec->bufio, block, buf, ioprio);
7478
if (IS_ERR(res)) {
@@ -128,11 +132,12 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_io *io,
128132
{
129133
int r, corrected = 0, res;
130134
struct dm_buffer *buf;
131-
unsigned int n, i, offset;
132-
u8 *par, *block;
135+
unsigned int n, i, offset, par_buf_offset = 0;
136+
u8 *par, *block, par_buf[DM_VERITY_FEC_RSM - DM_VERITY_FEC_MIN_RSN];
133137
struct bio *bio = dm_bio_from_per_bio_data(io, v->ti->per_io_data_size);
134138

135-
par = fec_read_parity(v, rsb, block_offset, &offset, &buf, bio_prio(bio));
139+
par = fec_read_parity(v, rsb, block_offset, &offset,
140+
par_buf_offset, &buf, bio_prio(bio));
136141
if (IS_ERR(par))
137142
return PTR_ERR(par);
138143

@@ -142,7 +147,8 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_io *io,
142147
*/
143148
fec_for_each_buffer_rs_block(fio, n, i) {
144149
block = fec_buffer_rs_block(v, fio, n, i);
145-
res = fec_decode_rs8(v, fio, block, &par[offset], neras);
150+
memcpy(&par_buf[par_buf_offset], &par[offset], v->fec->roots - par_buf_offset);
151+
res = fec_decode_rs8(v, fio, block, par_buf, neras);
146152
if (res < 0) {
147153
r = res;
148154
goto error;
@@ -155,12 +161,21 @@ static int fec_decode_bufs(struct dm_verity *v, struct dm_verity_io *io,
155161
if (block_offset >= 1 << v->data_dev_block_bits)
156162
goto done;
157163

158-
/* read the next block when we run out of parity bytes */
159-
offset += v->fec->roots;
164+
/* Read the next block when we run out of parity bytes */
165+
offset += (v->fec->roots - par_buf_offset);
166+
/* Check if parity bytes are split between blocks */
167+
if (offset < v->fec->io_size && (offset + v->fec->roots) > v->fec->io_size) {
168+
par_buf_offset = v->fec->io_size - offset;
169+
memcpy(par_buf, &par[offset], par_buf_offset);
170+
offset += par_buf_offset;
171+
} else
172+
par_buf_offset = 0;
173+
160174
if (offset >= v->fec->io_size) {
161175
dm_bufio_release(buf);
162176

163-
par = fec_read_parity(v, rsb, block_offset, &offset, &buf, bio_prio(bio));
177+
par = fec_read_parity(v, rsb, block_offset, &offset,
178+
par_buf_offset, &buf, bio_prio(bio));
164179
if (IS_ERR(par))
165180
return PTR_ERR(par);
166181
}
@@ -724,10 +739,7 @@ int verity_fec_ctr(struct dm_verity *v)
724739
return -E2BIG;
725740
}
726741

727-
if ((f->roots << SECTOR_SHIFT) & ((1 << v->data_dev_block_bits) - 1))
728-
f->io_size = 1 << v->data_dev_block_bits;
729-
else
730-
f->io_size = v->fec->roots << SECTOR_SHIFT;
742+
f->io_size = 1 << v->data_dev_block_bits;
731743

732744
f->bufio = dm_bufio_client_create(f->dev->bdev,
733745
f->io_size,

0 commit comments

Comments
 (0)