dm-crypt: recheck the integrity tag after a failure

commit 42e15d12070b4ff9af2b980f1b65774c2dab0507 upstream.

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: Mikulas Patocka <mpatocka@redhat.com>
Cc: stable@vger.kernel.org
Signed-off-by: Mike Snitzer <snitzer@kernel.org>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
This commit is contained in:
Mikulas Patocka 2024-02-19 21:31:11 +01:00 committed by Greg Kroah-Hartman
parent 287abdcb9e
commit 5583552eec
1 changed files with 73 additions and 16 deletions

View File

@ -61,6 +61,8 @@ struct convert_context {
struct skcipher_request *req;
struct aead_request *req_aead;
} r;
bool aead_recheck;
bool aead_failed;
};
@ -81,6 +83,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;
@ -1365,10 +1369,13 @@ static int crypt_convert_block_aead(struct crypt_config *cc,
if (r == -EBADMSG) {
sector_t s = le64_to_cpu(*sector);
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);
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)
@ -1724,6 +1731,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;
@ -1735,6 +1744,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.
@ -1748,6 +1759,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);
@ -1783,15 +1803,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) {
@ -1812,6 +1836,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
@ -2074,6 +2114,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);
}
@ -2103,11 +2151,17 @@ static void kcryptd_crypt_read_convert(struct dm_crypt_io *io)
crypt_inc_pending(io);
crypt_convert_init(cc, &io->ctx, io->base_bio, io->base_bio,
io->sector);
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);
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
@ -2150,10 +2204,13 @@ static void kcryptd_async_done(struct crypto_async_request *async_req,
if (error == -EBADMSG) {
sector_t s = le64_to_cpu(*org_sector_of_dmreq(cc, dmreq));
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);
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;
@ -3079,7 +3136,7 @@ static int crypt_ctr_optional(struct dm_target *ti, unsigned int argc, char **ar
sval = strchr(opt_string + strlen("integrity:"), ':') + 1;
if (!strcasecmp(sval, "aead")) {
set_bit(CRYPT_MODE_INTEGRITY_AEAD, &cc->cipher_flags);
} else if (strcasecmp(sval, "none")) {
} else if (strcasecmp(sval, "none")) {
ti->error = "Unknown integrity profile";
return -EINVAL;
}