Commit 42e15d12 authored by Mikulas Patocka's avatar Mikulas Patocka Committed by Mike Snitzer
Browse files

dm-crypt: recheck the integrity tag after a failure 

If a userspace process reads (with O_DIRECT) multiple blocks into the same
buffer, dm-crypt reports an authentication error [1]. The error is
reported in a log and it may cause RAID leg being kicked out of the
array.

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

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



Signed-off-by: default avatarMikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: default avatarMike Snitzer <snitzer@kernel.org>
parent 50c70240

drivers/md/dm-crypt.c

+73 −16
Original line number Diff line number Diff line
@@ -62,6 +62,8 @@ struct convert_context { 
		struct skcipher_request *req;

		struct aead_request *req_aead;

	} r;

	bool aead_recheck;

	bool aead_failed;



};
@@ -82,6 +84,8 @@ struct dm_crypt_io { 
	blk_status_t error;

	sector_t sector;



	struct bvec_iter saved_bi_iter;



	struct rb_node rb_node;

} CRYPTO_MINALIGN_ATTR;
@@ -1370,11 +1374,14 @@ static int crypt_convert_block_aead(struct crypt_config *cc, 
	if (r == -EBADMSG) {

		sector_t s = le64_to_cpu(*sector);



		ctx->aead_failed = true;

		if (ctx->aead_recheck) {

			DMERR_LIMIT("%pg: INTEGRITY AEAD ERROR, sector %llu",

				    ctx->bio_in->bi_bdev, s);

			dm_audit_log_bio(DM_MSG_PREFIX, "integrity-aead",

					 ctx->bio_in, s, 0);

		}

	}



	if (!r && cc->iv_gen_ops && cc->iv_gen_ops->post)

		r = cc->iv_gen_ops->post(cc, org_iv, dmreq);
@@ -1757,6 +1764,8 @@ static void crypt_io_init(struct dm_crypt_io *io, struct crypt_config *cc, 
	io->base_bio = bio;

	io->sector = sector;

	io->error = 0;

	io->ctx.aead_recheck = false;

	io->ctx.aead_failed = false;

	io->ctx.r.req = NULL;

	io->integrity_metadata = NULL;

	io->integrity_metadata_from_pool = false;
@@ -1768,6 +1777,8 @@ static void crypt_inc_pending(struct dm_crypt_io *io) 
	atomic_inc(&io->io_pending);

}



static void kcryptd_queue_read(struct dm_crypt_io *io);



/*

 * One of the bios was finished. Check for completion of

 * the whole request and correctly clean up the buffer.
@@ -1781,6 +1792,15 @@ static void crypt_dec_pending(struct dm_crypt_io *io) 
	if (!atomic_dec_and_test(&io->io_pending))

		return;



	if (likely(!io->ctx.aead_recheck) && unlikely(io->ctx.aead_failed) &&

	    cc->on_disk_tag_size && bio_data_dir(base_bio) == READ) {

		io->ctx.aead_recheck = true;

		io->ctx.aead_failed = false;

		io->error = 0;

		kcryptd_queue_read(io);

		return;

	}



	if (io->ctx.r.req)

		crypt_free_req(cc, io->ctx.r.req, base_bio);
@@ -1816,15 +1836,19 @@ static void crypt_endio(struct bio *clone) 
	struct dm_crypt_io *io = clone->bi_private;

	struct crypt_config *cc = io->cc;

	unsigned int rw = bio_data_dir(clone);

	blk_status_t error;

	blk_status_t error = clone->bi_status;



	if (io->ctx.aead_recheck && !error) {

		kcryptd_queue_crypt(io);

		return;

	}



	/*

	 * free the processed pages

	 */

	if (rw == WRITE)

	if (rw == WRITE || io->ctx.aead_recheck)

		crypt_free_buffer_pages(cc, clone);



	error = clone->bi_status;

	bio_put(clone);



	if (rw == READ && !error) {
@@ -1845,6 +1869,22 @@ static int kcryptd_io_read(struct dm_crypt_io *io, gfp_t gfp) 
	struct crypt_config *cc = io->cc;

	struct bio *clone;



	if (io->ctx.aead_recheck) {

		if (!(gfp & __GFP_DIRECT_RECLAIM))

			return 1;

		crypt_inc_pending(io);

		clone = crypt_alloc_buffer(io, io->base_bio->bi_iter.bi_size);

		if (unlikely(!clone)) {

			crypt_dec_pending(io);

			return 1;

		}

		clone->bi_iter.bi_sector = cc->start + io->sector;

		crypt_convert_init(cc, &io->ctx, clone, clone, io->sector);

		io->saved_bi_iter = clone->bi_iter;

		dm_submit_bio_remap(io->base_bio, clone);

		return 0;

	}



	/*

	 * We need the original biovec array in order to decrypt the whole bio

	 * data *afterwards* -- thanks to immutable biovecs we don't need to
@@ -2113,6 +2153,14 @@ static void kcryptd_crypt_write_convert(struct dm_crypt_io *io) 


static void kcryptd_crypt_read_done(struct dm_crypt_io *io)

{

	if (io->ctx.aead_recheck) {

		if (!io->error) {

			io->ctx.bio_in->bi_iter = io->saved_bi_iter;

			bio_copy_data(io->base_bio, io->ctx.bio_in);

		}

		crypt_free_buffer_pages(io->cc, io->ctx.bio_in);

		bio_put(io->ctx.bio_in);

	}

	crypt_dec_pending(io);

}
@@ -2142,11 +2190,17 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io) 


	crypt_inc_pending(io);



	if (io->ctx.aead_recheck) {

		io->ctx.cc_sector = io->sector + cc->iv_offset;

		r = crypt_convert(cc, &io->ctx,

				  test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags), true);

	} else {

		crypt_convert_init(cc, &io->ctx, io->base_bio, io->base_bio,

				   io->sector);



		r = crypt_convert(cc, &io->ctx,

				  test_bit(DM_CRYPT_NO_READ_WORKQUEUE, &cc->flags), true);

	}

	/*

	 * Crypto API backlogged the request, because its queue was full

	 * and we're in softirq context, so continue from a workqueue
@@ -2188,10 +2242,13 @@ static void kcryptd_async_done(void *data, int error) 
	if (error == -EBADMSG) {

		sector_t s = le64_to_cpu(*org_sector_of_dmreq(cc, dmreq));



		ctx->aead_failed = true;

		if (ctx->aead_recheck) {

			DMERR_LIMIT("%pg: INTEGRITY AEAD ERROR, sector %llu",

				    ctx->bio_in->bi_bdev, s);

			dm_audit_log_bio(DM_MSG_PREFIX, "integrity-aead",

					 ctx->bio_in, s, 0);

		}

		io->error = BLK_STS_PROTECTION;

	} else if (error < 0)

		io->error = BLK_STS_IOERR;