dm-verity-fec: fix reading parity bytes split across blocks (take 3)

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>

diff --git a/drivers/md/dm-verity-fec.c b/drivers/md/dm-verity-fec.c
index 1e776e0d6be5cdc00cb2c57cf05c2c0b2a6a95a2..e5d38bb3f16f1341bd333dbcc0d7ff89517de7a1 100644 (file)
--- a/drivers/md/dm-verity-fec.c
+++ b/drivers/md/dm-verity-fec.c
@@ -33,36 +33,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 allocated buffer. */
 #define fec_for_each_buffer(io, __i) \
        for (__i = 0; __i < (io)->nbufs; __i++)
@@ -102,15 +72,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
@@ -118,8 +102,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);
@@ -134,26 +142,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;