dm-verity: recheck the hash after a failure

If a userspace process reads (with O_DIRECT) multiple blocks into the same
buffer, dm-verity reports an error [1].

This commit fixes dm-verity, so that if hash verification fails, the data
is read again into a kernel buffer (where userspace can't modify it) and
the hash is rechecked. If the recheck succeeds, the content of the kernel
buffer is copied into the user buffer; if the recheck fails, an error is
reported.

[1] https://people.redhat.com/~mpatocka/testcases/blk-auth-modify/read2.c

Signed-off-by: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Mike Snitzer <snitzer@kernel.org>

diff --git a/drivers/md/dm-verity-target.c b/drivers/md/dm-verity-target.c
index 82662f5769c4af7f5456fc97e044c63574162060..224469e1efbcca779431c5e25772524960743dba 100644 (file)
--- a/drivers/md/dm-verity-target.c
+++ b/drivers/md/dm-verity-target.c
@@ -482,6 +482,63 @@ int verity_for_bv_block(struct dm_verity *v, struct dm_verity_io *io,
        return 0;
 }
 
+static int verity_recheck_copy(struct dm_verity *v, struct dm_verity_io *io,
+                              u8 *data, size_t len)
+{
+       memcpy(data, io->recheck_buffer, len);
+       io->recheck_buffer += len;
+
+       return 0;
+}
+
+static int verity_recheck(struct dm_verity *v, struct dm_verity_io *io,
+                         struct bvec_iter start, sector_t cur_block)
+{
+       struct page *page;
+       void *buffer;
+       int r;
+       struct dm_io_request io_req;
+       struct dm_io_region io_loc;
+
+       page = mempool_alloc(&v->recheck_pool, GFP_NOIO);
+       buffer = page_to_virt(page);
+
+       io_req.bi_opf = REQ_OP_READ;
+       io_req.mem.type = DM_IO_KMEM;
+       io_req.mem.ptr.addr = buffer;
+       io_req.notify.fn = NULL;
+       io_req.client = v->io;
+       io_loc.bdev = v->data_dev->bdev;
+       io_loc.sector = cur_block << (v->data_dev_block_bits - SECTOR_SHIFT);
+       io_loc.count = 1 << (v->data_dev_block_bits - SECTOR_SHIFT);
+       r = dm_io(&io_req, 1, &io_loc, NULL);
+       if (unlikely(r))
+               goto free_ret;
+
+       r = verity_hash(v, verity_io_hash_req(v, io), buffer,
+                       1 << v->data_dev_block_bits,
+                       verity_io_real_digest(v, io), true);
+       if (unlikely(r))
+               goto free_ret;
+
+       if (memcmp(verity_io_real_digest(v, io),
+                  verity_io_want_digest(v, io), v->digest_size)) {
+               r = -EIO;
+               goto free_ret;
+       }
+
+       io->recheck_buffer = buffer;
+       r = verity_for_bv_block(v, io, &start, verity_recheck_copy);
+       if (unlikely(r))
+               goto free_ret;
+
+       r = 0;
+free_ret:
+       mempool_free(page, &v->recheck_pool);
+
+       return r;
+}
+
 static int verity_bv_zero(struct dm_verity *v, struct dm_verity_io *io,
                          u8 *data, size_t len)
 {
@@ -508,9 +565,7 @@ static int verity_verify_io(struct dm_verity_io *io)
 {
        bool is_zero;
        struct dm_verity *v = io->v;
-#if defined(CONFIG_DM_VERITY_FEC)
        struct bvec_iter start;
-#endif
        struct bvec_iter iter_copy;
        struct bvec_iter *iter;
        struct crypto_wait wait;
@@ -561,10 +616,7 @@ static int verity_verify_io(struct dm_verity_io *io)
                if (unlikely(r < 0))
                        return r;
 
-#if defined(CONFIG_DM_VERITY_FEC)
-               if (verity_fec_is_enabled(v))
-                       start = *iter;
-#endif
+               start = *iter;
                r = verity_for_io_block(v, io, iter, &wait);
                if (unlikely(r < 0))
                        return r;
@@ -586,6 +638,10 @@ static int verity_verify_io(struct dm_verity_io *io)
                         * tasklet since it may sleep, so fallback to work-queue.
                         */
                        return -EAGAIN;
+               } else if (verity_recheck(v, io, start, cur_block) == 0) {
+                       if (v->validated_blocks)
+                               set_bit(cur_block, v->validated_blocks);
+                       continue;
 #if defined(CONFIG_DM_VERITY_FEC)
                } else if (verity_fec_decode(v, io, DM_VERITY_BLOCK_TYPE_DATA,
                                             cur_block, NULL, &start) == 0) {
@@ -941,6 +997,10 @@ static void verity_dtr(struct dm_target *ti)
        if (v->verify_wq)
                destroy_workqueue(v->verify_wq);
 
+       mempool_exit(&v->recheck_pool);
+       if (v->io)
+               dm_io_client_destroy(v->io);
+
        if (v->bufio)
                dm_bufio_client_destroy(v->bufio);
 
@@ -1379,6 +1439,20 @@ static int verity_ctr(struct dm_target *ti, unsigned int argc, char **argv)
        }
        v->hash_blocks = hash_position;
 
+       r = mempool_init_page_pool(&v->recheck_pool, 1, 0);
+       if (unlikely(r)) {
+               ti->error = "Cannot allocate mempool";
+               goto bad;
+       }
+
+       v->io = dm_io_client_create();
+       if (IS_ERR(v->io)) {
+               r = PTR_ERR(v->io);
+               v->io = NULL;
+               ti->error = "Cannot allocate dm io";
+               goto bad;
+       }
+
        v->bufio = dm_bufio_client_create(v->hash_dev->bdev,
                1 << v->hash_dev_block_bits, 1, sizeof(struct buffer_aux),
                dm_bufio_alloc_callback, NULL,

diff --git a/drivers/md/dm-verity.h b/drivers/md/dm-verity.h
index f3f6070084196825f21dcc17947f67974a90cbde..4620a98c99561803431dc7674639adc3a2d422db 100644 (file)
--- a/drivers/md/dm-verity.h
+++ b/drivers/md/dm-verity.h
@@ -11,6 +11,7 @@
 #ifndef DM_VERITY_H
 #define DM_VERITY_H
 
+#include <linux/dm-io.h>
 #include <linux/dm-bufio.h>
 #include <linux/device-mapper.h>
 #include <linux/interrupt.h>
@@ -68,6 +69,9 @@ struct dm_verity {
        unsigned long *validated_blocks; /* bitset blocks validated */
 
        char *signature_key_desc; /* signature keyring reference */
+
+       struct dm_io_client *io;
+       mempool_t recheck_pool;
 };
 
 struct dm_verity_io {
@@ -84,6 +88,8 @@ struct dm_verity_io {
 
        struct work_struct work;
 
+       char *recheck_buffer;
+
        /*
         * Three variably-size fields follow this struct:
         *